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.

Sign up for GitHub

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

Jump to bottom

updated controller-manager-library for startup check issue #34

Merged
merged 1 commit into from
Jul 10, 2020
Merged

updated controller-manager-library for startup check issue #34

merged 1 commit into from
Jul 10, 2020

Conversation

MartinWeindel
Copy link
Member

What this PR does / why we need it:
Fixes an issue in the controller-manager-library on controller startup with checks of watcher on wrong cluster.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Release note:

bug fix for watches check on controller startup in the controller-manager-library

@MartinWeindel MartinWeindel requested a review from a team as a code owner July 9, 2020 14:29
@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) 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 Jul 9, 2020
@ialidzhikov
Copy link
Member

ialidzhikov commented Jul 9, 2020 •
edited
Loading

This PR seems to fix the Issuer.cert.gardener.cloud not known issue.

Now I also see the following logs

E0709 14:52:43.176146       1 leaderelection.go:330] error retrieving resource lock shoot--foo--bar/cert-controller-manager: configmaps "cert-controller-manager" is forbidden: User "gardener.cloud:system:cert-management" cannot get resource "configmaps" in API group "" in the namespace "shoot--foo--bar"
E0709 14:52:45.445372       1 leaderelection.go:330] error retrieving resource lock shoot--foo--bar/cert-controller-manager: configmaps "cert-controller-manager" is forbidden: User "gardener.cloud:system:cert-management" cannot get resource "configmaps" in API group "" in the namespace "shoot--foo--bar"

Not sure where adaptation is needed - here or in shoot-cert-service repo?

@ialidzhikov
Copy link
Member

It looks like with #30 RequireLease() is added to the controllers.

@MartinWeindel
Copy link
Member Author

Regarding the error retrieving resource lock: this must be added in the charts here. I take a look at it and add it to this fix.
@ialidzhikov Thanks for bringing this up!

@MartinWeindel
Copy link
Member Author

I'm wrong, this must be added to the charts in the shoot-cert-service repo

ialidzhikov
ialidzhikov approved these changes Jul 9, 2020
View reviewed changes
Copy link
Member

@ialidzhikov ialidzhikov left a comment

Choose a reason for hiding this comment

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

Thanks for this PR! As I already wrote earlier - I tested it and works well.

@ialidzhikov
Copy link
Member

/lgtm

@gardener-robot gardener-robot added the reviewed/lgtm Has approval for merging label Jul 9, 2020
@ialidzhikov
Copy link
Member

Assigning @mandelsoft for approval.

/assign @mandelsoft

@ialidzhikov
Copy link
Member

I'm wrong, this must be added to the charts in the shoot-cert-service repo

@MartinWeindel , I proposed gardener/gardener-extension-shoot-cert-service#31. Could you please check?

@ialidzhikov
Copy link
Member

We need also to cherry-pick this into release-v0.3 and release v0.3.1.

@ialidzhikov
Copy link
Member

/kind bug

@MartinWeindel MartinWeindel merged commit 8d8efd0 into master Jul 10, 2020
@MartinWeindel MartinWeindel deleted the fix-startup-check branch July 10, 2020 06:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ialidzhikov ialidzhikov ialidzhikov approved these changes

Assignees

@mandelsoft mandelsoft

Labels
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
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@MartinWeindel @ialidzhikov @mandelsoft @gardener-robot-ci-3 @gardener-robot