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

avoid conflicting updates of the status.state of the extension resource by the replication controller #56

Merged
merged 1 commit into from
Jun 2, 2021

Conversation

MartinWeindel
Copy link
Member

in parallel

How to categorize this PR?

/area control-plane
/kind bug

What this PR does / why we need it:
The shoot-dns-service can loose its leader election if there is a high workload because of conflicting updates on replicating changed DNS entries into the DNS state of the extension object.
The replication controller, responsible for replication the entries into the status.state of the extension resource, now locks the namespace to avoid conflicting updates. If the namespace is already locked, the reconciliation is delay for about 1-2s.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Release note:

avoid conflicting updates of the `status.state` of the extension resource by the replication controller

@MartinWeindel MartinWeindel requested review from a team as code owners May 26, 2021 14:29
@gardener-robot-ci-2 gardener-robot-ci-2 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 May 26, 2021
@gardener-robot gardener-robot added area/control-plane Control plane related kind/bug Bug needs/review Needs review size/m Size of pull request is medium (see gardener-robot robot/bots/size.py) labels May 26, 2021
@MartinWeindel MartinWeindel force-pushed the fix-update-conflict-on-extension branch from 5152297 to d06e583 Compare May 27, 2021 07:16
@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 May 27, 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 May 27, 2021
Copy link
Member

@timuthy timuthy left a comment

Choose a reason for hiding this comment

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

I left one comment in the review and I'm also missing unit tests for StringLock. Can you add them?

pkg/controller/replication/stringslock.go Outdated Show resolved Hide resolved
@gardener-robot gardener-robot added the needs/changes Needs (more) changes label May 28, 2021
@MartinWeindel MartinWeindel force-pushed the fix-update-conflict-on-extension branch from d06e583 to 466ec7a Compare May 28, 2021 10:20
@gardener-robot-ci-3 gardener-robot-ci-3 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 May 28, 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 May 28, 2021
@MartinWeindel
Copy link
Member Author

unit test for stringslock has been added

@MartinWeindel MartinWeindel force-pushed the fix-update-conflict-on-extension branch from 466ec7a to cfc97a8 Compare May 28, 2021 10:27
@gardener-robot-ci-3 gardener-robot-ci-3 added reviewed/ok-to-test Has 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 May 28, 2021
@MartinWeindel MartinWeindel force-pushed the fix-update-conflict-on-extension branch from cfc97a8 to 8354233 Compare May 28, 2021 10:28
@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) 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 May 28, 2021
@MartinWeindel MartinWeindel force-pushed the fix-update-conflict-on-extension branch from 8354233 to 26dd5ce Compare May 28, 2021 11:11
@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 May 28, 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 May 28, 2021
@gardener-robot
Copy link

@MartinWeindel You have pull request review with status CHANGES_REQUESTED, please check

@MartinWeindel MartinWeindel force-pushed the fix-update-conflict-on-extension branch from 26dd5ce to f82760a Compare May 31, 2021 14:50
@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 May 31, 2021
@gardener-robot-ci-2 gardener-robot-ci-2 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 May 31, 2021
Copy link
Member

@mandelsoft mandelsoft left a comment

Choose a reason for hiding this comment

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

Switch to InjectAPIReader in Environment (Env)

@MartinWeindel MartinWeindel force-pushed the fix-update-conflict-on-extension branch from f82760a to b99b4b4 Compare June 1, 2021 11:46
@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 Jun 1, 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 Jun 1, 2021
@MartinWeindel MartinWeindel force-pushed the fix-update-conflict-on-extension branch from b99b4b4 to 2771aee Compare June 1, 2021 15:37
@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 Jun 1, 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 Jun 1, 2021
Copy link
Member

@timuthy timuthy left a comment

Choose a reason for hiding this comment

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

/lgtm

@gardener-robot gardener-robot added reviewed/lgtm Has approval for merging and removed needs/changes Needs (more) changes needs/review Needs review labels Jun 2, 2021
@MartinWeindel MartinWeindel merged commit 1bd931f into master Jun 2, 2021
@MartinWeindel MartinWeindel deleted the fix-update-conflict-on-extension branch June 2, 2021 06:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/control-plane Control plane related kind/bug Bug needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) reviewed/lgtm Has approval for merging size/m Size of pull request is medium (see gardener-robot robot/bots/size.py)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants