Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GCU] Disallowing DeleteInsteadOfReplaceMoveExtender from generating delete whole config move #2006

Merged
merged 1 commit into from
Jan 13, 2022

Conversation

ghooo
Copy link
Contributor

@ghooo ghooo commented Jan 12, 2022

What I did

Not generating delete whole config move because it is not allowed by JsonPatch library and it is not possible for ConfigDb to be equal to NULL.

This solves the first problem in issue #2000 (comment)

How I did it

  • Verified UpperLevelMoveExtender does not generate delete whole config move
  • Modified DeleteInsteadOfReplaceMoveExtender to not generate delete whole config move
  • DeleteRefsMoveExtender cannot generate delete whole config move, because it deletes the referrer leafs

How to verify it

unit-test

Previous command output (if the output of a command-line utility has changed)

New command output (if the output of a command-line utility has changed)

@@ -2811,5 +2811,88 @@
}
]
]
},
"ADDING_LOOPBACK0_VRF_NAME__DELETES_LOOPBACK0_AND_IPS_DOES_NOT_AFFECT_OTHER_TABLES": {
Copy link
Contributor Author

@ghooo ghooo Jan 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test case related to issue #2000 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolve the issue

@wen587
Copy link
Contributor

wen587 commented Jan 13, 2022

Verified locally. Vrf change can pass now.

@@ -2811,5 +2811,88 @@
}
]
]
},
"ADDING_LOOPBACK0_VRF_NAME__DELETES_LOOPBACK0_AND_IPS_DOES_NOT_AFFECT_OTHER_TABLES": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolve the issue

@ghooo ghooo merged commit fc0ecb6 into sonic-net:master Jan 13, 2022
judyjoseph pushed a commit that referenced this pull request Jan 17, 2022
…delete whole config move (#2006)

#### What I did
Not generating delete whole config move because it is not allowed by JsonPatch library and it is not possible for ConfigDb to be equal to NULL.

This solves the first problem in issue #2000 (comment)

#### How I did it
- Verified UpperLevelMoveExtender does not generate delete whole config move
- Modified DeleteInsteadOfReplaceMoveExtender to not generate delete whole config move
- DeleteRefsMoveExtender cannot generate delete whole config move, because it deletes the referrer leafs

#### How to verify it
unit-test

#### Previous command output (if the output of a command-line utility has changed)

#### New command output (if the output of a command-line utility has changed)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants