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

Added mutex lock before every update of ETCD instance. #163

Merged
merged 1 commit into from
Apr 22, 2021

Conversation

abdasgupta
Copy link
Contributor

Sometimes the following error is coming while running the controllers:

Expected success, but got an error:
      <*errors.StatusError | 0xc0002314a0>: {
          ErrStatus: {
              TypeMeta: {Kind: "", APIVersion: ""},
              ListMeta: {
                  SelfLink: "",
                  ResourceVersion: "",
                  Continue: "",
                  RemainingItemCount: nil,
              },
              Status: "Failure",
              Message: "Operation cannot be fulfilled on statefulsets.apps \"foo3\": the object has been modified; please apply your changes to the latest version and try again",
              Reason: "Conflict",
              Details: {Name: "foo3", Group: "apps", Kind: "statefulsets", UID: "", Causes: nil, RetryAfterSeconds: 0},
              Code: 409,
          },
      }
      Operation cannot be fulfilled on statefulsets.apps "foo3": the object has been modified; please apply your changes to the latest version and try again

Though this error is harmless as reconciler will reque after the error, but it is due to the fact that both the controllers(ETCD controller, custodian controller) are trying to update same ETCD resource. So to avoid data race condition, I added mutex lock/unlock before ETCD update operation.

@abdasgupta abdasgupta requested a review from a team as a code owner April 21, 2021 18:09
@gardener-robot gardener-robot added needs/review Needs review size/s Size of pull request is small (see gardener-robot robot/bots/size.py) labels Apr 21, 2021
@gardener-robot-ci-1 gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Apr 21, 2021
@gardener-robot-ci-1 gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Apr 22, 2021
@gardener-robot-ci-3 gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Apr 22, 2021
@gardener-robot-ci-2 gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Apr 22, 2021
@gardener-robot-ci-1 gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Apr 22, 2021
Copy link
Contributor

@shreyas-s-rao shreyas-s-rao left a comment

Choose a reason for hiding this comment

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

@abdasgupta thanks for fixing the race conditions in the tests. LGTM

@shreyas-s-rao shreyas-s-rao merged commit a4c3b7d into gardener:master Apr 22, 2021
@abdasgupta abdasgupta deleted the working branch April 22, 2021 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) needs/review Needs review size/s Size of pull request is small (see gardener-robot robot/bots/size.py)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants