-
Notifications
You must be signed in to change notification settings - Fork 6.8k
fix: defensive approach to ignore the already exists error during the creation of k8s resources #26093
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
base: master
Are you sure you want to change the base?
fix: defensive approach to ignore the already exists error during the creation of k8s resources #26093
Conversation
…tion of resources Signed-off-by: Patroklos Papapetrou <ppapapetrou76@gmail.com>
…tion of resources Signed-off-by: Patroklos Papapetrou <ppapapetrou76@gmail.com>
🔴 Preview Environment stopped on BunnyshellSee: Environment Details | Pipeline Logs Available commands (reply to this comment):
|
| _, getErr := dynamicResource.Get(ctx, obj.GetName(), metav1.GetOptions{}) | ||
| if getErr == nil { | ||
| // Object is present — treat the transient AlreadyExists as success. | ||
| k.log.Info(fmt.Sprintf("Create reported AlreadyExists for %s/%s but object exists - treating as success", obj.GetKind(), obj.GetName())) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we check that is not a genuine AlreadyExist error - another object with same gvk and name already exists for some time (probably need additionally check creation time of the existing object or compare attributes)?
In other words - is this function always called after a check that the object being created does not exist, so this race condition is the only possible cause for this error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dudinea no this race condition might not be the only root cause and that's why I described this a defensive code change because it can make the system handle such errors ( from any given root cause ) with a more smooth way without returning unwanted sync errors to the users
…sts addition Signed-off-by: Patroklos Papapetrou <ppapapetrou76@gmail.com>
f4e5d06 to
6a51f06
Compare
Signed-off-by: Patroklos Papapetrou <ppapapetrou76@gmail.com>
Signed-off-by: Patroklos Papapetrou <ppapapetrou76@gmail.com>
This PR introduces a defensive mechanism to avoid the "already exists" error messages when syncing apps with the
replaceflag onIt also adds a unit test to test this functionality.
This fix address the symptom and not the root cause - I will follow-up with another PR and implementation to fix also the root cause
Closes #24154
Checklist: