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 predicates to the controller mgr with option reconcile only if annotation present. #33

Merged
merged 1 commit into from
Mar 29, 2020

Conversation

georgekuruvillak
Copy link

@georgekuruvillak georgekuruvillak commented Mar 25, 2020

What this PR does / why we need it:
This PR does the following:

  • remove lastError after successful reconciliation
  • fix the labels/selector issue in the statefulset manifest
  • introduce .status.observedGeneration
  • only reconcile if gardener.cloud/operation=reconcile and oldMeta.generation != newMeta.generation
    Which issue(s) this PR fixes:
    Fixes #

Special notes for your reviewer:

Release note:

* Removed .status.lastError after successful reconciliation
* Fixed the labels/selector issue in the statefulset manifest
* Introduced .status.observedGeneration
* Added logic to only reconcile if `gardener.cloud/operation=reconcil`e and `oldMeta.generation != newMeta.generation` or `.status.LastError != nil`

@georgekuruvillak georgekuruvillak requested a review from a team as a code owner March 25, 2020 17:35
@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 Mar 25, 2020
@gardener-robot-ci-3 gardener-robot-ci-3 added 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 Mar 25, 2020
Copy link
Collaborator

@amshuman-kr amshuman-kr left a comment

Choose a reason for hiding this comment

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

Can you please updated the release notes?

@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 Mar 26, 2020
@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 Mar 26, 2020
@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 Mar 26, 2020
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.

Just a typo

api/v1alpha1/etcd_types.go Outdated Show resolved Hide resolved
controllers/etcd_controller.go Outdated Show resolved Hide resolved
pkg/predicate/predicate.go Outdated Show resolved Hide resolved
@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 Mar 26, 2020
@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 Mar 26, 2020
@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 Mar 26, 2020
@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 Mar 26, 2020
@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 Mar 26, 2020
@georgekuruvillak georgekuruvillak changed the title [WIP] Added predicates to the controller mgr with option reconcile only if annotation present. Added predicates to the controller mgr with option reconcile only if annotation present. Mar 26, 2020
@georgekuruvillak georgekuruvillak changed the title Added predicates to the controller mgr with option reconcile only if annotation present. [WIP] Added predicates to the controller mgr with option reconcile only if annotation present. Mar 27, 2020
controllers/etcd_controller.go Outdated Show resolved Hide resolved
controllers/etcd_controller.go Outdated Show resolved Hide resolved
controllers/etcd_controller.go Outdated Show resolved Hide resolved
controllers/etcd_controller.go Outdated Show resolved Hide resolved
controllers/etcd_controller.go Outdated Show resolved Hide resolved
pkg/predicate/predicate.go Outdated Show resolved Hide resolved
pkg/predicate/mapper.go Outdated Show resolved Hide resolved
controllers/etcd_controller.go Show resolved Hide resolved
Copy link
Member

@rfranzke rfranzke left a comment

Choose a reason for hiding this comment

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

/lgtm

@rfranzke rfranzke changed the title [WIP] Added predicates to the controller mgr with option reconcile only if annotation present. Added predicates to the controller mgr with option reconcile only if annotation present. Mar 29, 2020
@rfranzke rfranzke requested a review from amshuman-kr March 29, 2020 11:06
Copy link
Collaborator

@amshuman-kr amshuman-kr left a comment

Choose a reason for hiding this comment

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

@georgekuruvillak I have make some small suggestions/change requests.

charts/etcd/templates/etcd-statefulset.yaml Show resolved Hide resolved
api/v1alpha1/etcd_types.go Outdated Show resolved Hide resolved
config/crd/bases/druid.gardener.cloud_etcds.yaml Outdated Show resolved Hide resolved
controllers/etcd_controller.go Show resolved Hide resolved
controllers/etcd_controller.go Outdated Show resolved Hide resolved
controllers/etcd_controller.go Show resolved Hide resolved
controllers/etcd_controller.go Outdated Show resolved Hide resolved
pkg/predicate/predicate.go Outdated Show resolved Hide resolved
@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 Mar 29, 2020
@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 Mar 29, 2020
@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 Mar 29, 2020
@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 Mar 29, 2020
@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 Mar 29, 2020
@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 Mar 29, 2020
@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 Mar 29, 2020
@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 Mar 29, 2020
@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 Mar 29, 2020
@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 Mar 29, 2020
@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 Mar 29, 2020
… if annotation present.

Apply suggestions from code review

Co-Authored-By: Amshuman K R <[email protected]>
@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) 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 Mar 29, 2020
@rfranzke rfranzke requested a review from amshuman-kr March 29, 2020 15:31
Copy link
Collaborator

@amshuman-kr amshuman-kr 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 the changes!

@amshuman-kr amshuman-kr merged commit 534c30c into gardener:master Mar 29, 2020
@georgekuruvillak georgekuruvillak deleted the predicates_annotation branch March 29, 2020 15:55
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)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants