Use informers for CVO #7
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
abhinavdahiya
force-pushed
the
informers
branch
from
3bf53c3
to
8b88114
Compare
openshift-ci-robot
added
the
size/XXL
abhinavdahiya
force-pushed
the
informers
branch
from
8b88114
to
e55fa7c
Compare
crawford
reviewed
Aug 14, 2018
cmd/start.go
Outdated
| flag.Parse() | ||
|
|
||
| // To help debugging, immediately log version | ||
| glog.Infof("Version: %+v", version.Version) |
There was a problem hiding this comment.
Use version.String instead.
openshift-bot
added
the
needs-rebase
abhinavdahiya
force-pushed
the
informers
branch
from
e55fa7c
to
823684e
Compare
openshift-bot
removed
the
needs-rebase
|
/test all |
|
/refresh |
|
/test all |
|
/refresh |
|
/skip |
abhinavdahiya
force-pushed
the
informers
branch
from
823684e
to
9cf86f3
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: abhinavdahiya, crawford If they are not already assigned, you can assign the PR to them by writing The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
wking
added a commit
to wking/cluster-version-operator
that referenced
this pull request
Jan 16, 2020
… in required We claimed: if we have no required, then we don't care what someone else has set since this code landed in d9f6718 (lib: add lib for applying objects, 2018-08-14, openshift#7). But we do care in situations like [1], where a 4.3 -> 4.2 downgrade leaves 4.3 readiness probes in place for a 4.2 operator which has no /readyz. With this commit we still have a few other types where we claim to not care: $ git grep -hB1 'someone else has set' | grep func func ensureSecurityContextPtr(modified *bool, existing **corev1.SecurityContext, required *corev1.SecurityContext) { func ensureCapabilitiesPtr(modified *bool, existing **corev1.Capabilities, required *corev1.Capabilities) { func ensureAffinityPtr(modified *bool, existing **corev1.Affinity, required *corev1.Affinity) { func ensurePodSecurityContextPtr(modified *bool, existing **corev1.PodSecurityContext, required *corev1.PodSecurityContext) { func ensureSELinuxOptionsPtr(modified *bool, existing **corev1.SELinuxOptions, required *corev1.SELinuxOptions) { func setBoolPtr(modified *bool, existing **bool, required *bool) { func setInt64Ptr(modified *bool, existing **int64, required *int64) { but I'm leaving them alone until we have a clearer picture about whether we care about those specific properties or not. [1]: https://bugzilla.redhat.com/show_bug.cgi?id=1791863#c1
openshift-cherrypick-robot
pushed a commit
to openshift-cherrypick-robot/cluster-version-operator
that referenced
this pull request
Jan 17, 2020
… in required We claimed: if we have no required, then we don't care what someone else has set since this code landed in d9f6718 (lib: add lib for applying objects, 2018-08-14, openshift#7). But we do care in situations like [1], where a 4.3 -> 4.2 downgrade leaves 4.3 readiness probes in place for a 4.2 operator which has no /readyz. With this commit we still have a few other types where we claim to not care: $ git grep -hB1 'someone else has set' | grep func func ensureSecurityContextPtr(modified *bool, existing **corev1.SecurityContext, required *corev1.SecurityContext) { func ensureCapabilitiesPtr(modified *bool, existing **corev1.Capabilities, required *corev1.Capabilities) { func ensureAffinityPtr(modified *bool, existing **corev1.Affinity, required *corev1.Affinity) { func ensurePodSecurityContextPtr(modified *bool, existing **corev1.PodSecurityContext, required *corev1.PodSecurityContext) { func ensureSELinuxOptionsPtr(modified *bool, existing **corev1.SELinuxOptions, required *corev1.SELinuxOptions) { func setBoolPtr(modified *bool, existing **bool, required *bool) { func setInt64Ptr(modified *bool, existing **int64, required *int64) { but I'm leaving them alone until we have a clearer picture about whether we care about those specific properties or not. [1]: https://bugzilla.redhat.com/show_bug.cgi?id=1791863#c1
openshift-cherrypick-robot
pushed a commit
to openshift-cherrypick-robot/cluster-version-operator
that referenced
this pull request
Jan 17, 2020
… in required We claimed: if we have no required, then we don't care what someone else has set since this code landed in d9f6718 (lib: add lib for applying objects, 2018-08-14, openshift#7). But we do care in situations like [1], where a 4.3 -> 4.2 downgrade leaves 4.3 readiness probes in place for a 4.2 operator which has no /readyz. With this commit we still have a few other types where we claim to not care: $ git grep -hB1 'someone else has set' | grep func func ensureSecurityContextPtr(modified *bool, existing **corev1.SecurityContext, required *corev1.SecurityContext) { func ensureCapabilitiesPtr(modified *bool, existing **corev1.Capabilities, required *corev1.Capabilities) { func ensureAffinityPtr(modified *bool, existing **corev1.Affinity, required *corev1.Affinity) { func ensurePodSecurityContextPtr(modified *bool, existing **corev1.PodSecurityContext, required *corev1.PodSecurityContext) { func ensureSELinuxOptionsPtr(modified *bool, existing **corev1.SELinuxOptions, required *corev1.SELinuxOptions) { func setBoolPtr(modified *bool, existing **bool, required *bool) { func setInt64Ptr(modified *bool, existing **int64, required *int64) { but I'm leaving them alone until we have a clearer picture about whether we care about those specific properties or not. [1]: https://bugzilla.redhat.com/show_bug.cgi?id=1791863#c1
openshift-cherrypick-robot
pushed a commit
to openshift-cherrypick-robot/cluster-version-operator
that referenced
this pull request
Jan 17, 2020
… in required We claimed: if we have no required, then we don't care what someone else has set since this code landed in d9f6718 (lib: add lib for applying objects, 2018-08-14, openshift#7). But we do care in situations like [1], where a 4.3 -> 4.2 downgrade leaves 4.3 readiness probes in place for a 4.2 operator which has no /readyz. With this commit we still have a few other types where we claim to not care: $ git grep -hB1 'someone else has set' | grep func func ensureSecurityContextPtr(modified *bool, existing **corev1.SecurityContext, required *corev1.SecurityContext) { func ensureCapabilitiesPtr(modified *bool, existing **corev1.Capabilities, required *corev1.Capabilities) { func ensureAffinityPtr(modified *bool, existing **corev1.Affinity, required *corev1.Affinity) { func ensurePodSecurityContextPtr(modified *bool, existing **corev1.PodSecurityContext, required *corev1.PodSecurityContext) { func ensureSELinuxOptionsPtr(modified *bool, existing **corev1.SELinuxOptions, required *corev1.SELinuxOptions) { func setBoolPtr(modified *bool, existing **bool, required *bool) { func setInt64Ptr(modified *bool, existing **int64, required *int64) { but I'm leaving them alone until we have a clearer picture about whether we care about those specific properties or not. [1]: https://bugzilla.redhat.com/show_bug.cgi?id=1791863#c1
openshift-cherrypick-robot
pushed a commit
to openshift-cherrypick-robot/cluster-version-operator
that referenced
this pull request
Feb 24, 2020
… in required We claimed: if we have no required, then we don't care what someone else has set since this code landed in d9f6718 (lib: add lib for applying objects, 2018-08-14, openshift#7). But we do care in situations like [1], where a 4.3 -> 4.2 downgrade leaves 4.3 readiness probes in place for a 4.2 operator which has no /readyz. With this commit we still have a few other types where we claim to not care: $ git grep -hB1 'someone else has set' | grep func func ensureSecurityContextPtr(modified *bool, existing **corev1.SecurityContext, required *corev1.SecurityContext) { func ensureCapabilitiesPtr(modified *bool, existing **corev1.Capabilities, required *corev1.Capabilities) { func ensureAffinityPtr(modified *bool, existing **corev1.Affinity, required *corev1.Affinity) { func ensurePodSecurityContextPtr(modified *bool, existing **corev1.PodSecurityContext, required *corev1.PodSecurityContext) { func ensureSELinuxOptionsPtr(modified *bool, existing **corev1.SELinuxOptions, required *corev1.SELinuxOptions) { func setBoolPtr(modified *bool, existing **bool, required *bool) { func setInt64Ptr(modified *bool, existing **int64, required *int64) { but I'm leaving them alone until we have a clearer picture about whether we care about those specific properties or not. [1]: https://bugzilla.redhat.com/show_bug.cgi?id=1791863#c1
wking
added a commit
to wking/cluster-version-operator
that referenced
this pull request
Aug 5, 2020
This seems to have been copy-pasted from the machine-config operator in 6c8f725 (cvo: refactor to informers, 2018-08-14, openshift#7) and then from there over to the auto-update package with ad4bd6c (pkg: add autoupdate ctrl, 2018-08-22, openshift#11). Update to generic wording that is less likely to go stale.
wking
added a commit
to wking/cluster-version-operator
that referenced
this pull request
Aug 5, 2020
This seems to have been copy-pasted from the machine-config operator in 6c8f725 (cvo: refactor to informers, 2018-08-14, openshift#7) and then from there over to the auto-update package with ad4bd6c (pkg: add autoupdate ctrl, 2018-08-22, openshift#11). Update to generic wording that is less likely to go stale.
wking
added a commit
to wking/cluster-version-operator
that referenced
this pull request
Aug 5, 2020
This seems to have been copy-pasted from the machine-config operator in 6c8f725 (cvo: refactor to informers, 2018-08-14, openshift#7) and then from there over to the auto-update package with ad4bd6c (pkg: add autoupdate ctrl, 2018-08-22, openshift#11). Update to generic wording that is less likely to go stale.
wking
added a commit
to wking/cluster-version-operator
that referenced
this pull request
Sep 14, 2021
Since this package was created in d9f6718 (lib: add lib for applying objects, 2018-08-14, openshift#7), the volume(mount) merge logic has required manifest entries to exist, but has allowed in-cluster entries to persist without removal. That hasn't been a problem until [1]: 1. In 4.3, the autoscaler asked for a ca-cert volume mount, based on the cluster-autoscaler-operator-ca config map. 2. In 4.4, the autoscaler dropped those manifest entries [2]. 3. In 4.9, the autoscaler asked the CVO to remove the config map [3]. That lead some born-in 4.3 clusters to have crashlooping autoscalers, because the mount attempts kept failing on the missing config map. We couldn't think of a plausible reason why cluster admins would want to inject additional volume mounts in a CVO-managed pod configuration, so this commit removes that ability and begins clearing away any volume(mount) configuration that is not present in the reconciling manifest. Cluster administrators who do need to add additional mounts in an emergency are free to use ClusterVersion's spec.overrides to take control of a particular CVO-managed resource. This joins a series of similar previous tightenings, including 02bb9ba (lib/resourcemerge/core: Clear env and envFrom if unset in manifest, 2021-04-20, openshift#549) and ca299b8 (lib/resourcemerge: remove ports which are no longer required, 2020-02-13, openshift#322). [1]: https://bugzilla.redhat.com/show_bug.cgi?id=2002834 [2]: openshift/cluster-autoscaler-operator@f08589d#diff-547486373183980619528df695869ed32b80c18383bc16b57a5ee931bf0edd39L89 [3]: openshift/cluster-autoscaler-operator@9a7b3be#diff-d0cf785e044c611986a4d9bdd65bb373c86f9eb1c97bd3f105062184342a872dR4
wking
added a commit
to wking/cluster-version-operator
that referenced
this pull request
Sep 14, 2021
Since this package was created in d9f6718 (lib: add lib for applying objects, 2018-08-14, openshift#7), the volume(mount) merge logic has required manifest entries to exist, but has allowed in-cluster entries to persist without removal. That hasn't been a problem until [1]: 1. In 4.3, the autoscaler asked for a ca-cert volume mount, based on the cluster-autoscaler-operator-ca config map. 2. In 4.4, the autoscaler dropped those manifest entries [2]. 3. In 4.9, the autoscaler asked the CVO to remove the config map [3]. That lead some born-in 4.3 clusters to have crashlooping autoscalers, because the mount attempts kept failing on the missing config map. We couldn't think of a plausible reason why cluster admins would want to inject additional volume mounts in a CVO-managed pod configuration, so this commit removes that ability and begins clearing away any volume(mount) configuration that is not present in the reconciling manifest. Cluster administrators who do need to add additional mounts in an emergency are free to use ClusterVersion's spec.overrides to take control of a particular CVO-managed resource. This joins a series of similar previous tightenings, including 02bb9ba (lib/resourcemerge/core: Clear env and envFrom if unset in manifest, 2021-04-20, openshift#549) and ca299b8 (lib/resourcemerge: remove ports which are no longer required, 2020-02-13, openshift#322). [1]: https://bugzilla.redhat.com/show_bug.cgi?id=2002834 [2]: openshift/cluster-autoscaler-operator@f08589d#diff-547486373183980619528df695869ed32b80c18383bc16b57a5ee931bf0edd39L89 [3]: openshift/cluster-autoscaler-operator@9a7b3be#diff-d0cf785e044c611986a4d9bdd65bb373c86f9eb1c97bd3f105062184342a872dR4
wking
added a commit
to wking/cluster-version-operator
that referenced
this pull request
Sep 14, 2021
Since this package was created in d9f6718 (lib: add lib for applying objects, 2018-08-14, openshift#7), the volume(mount) merge logic has required manifest entries to exist, but has allowed in-cluster entries to persist without removal. That hasn't been a problem until [1]: 1. In 4.3, the autoscaler asked for a ca-cert volume mount, based on the cluster-autoscaler-operator-ca config map. 2. In 4.4, the autoscaler dropped those manifest entries [2]. 3. In 4.9, the autoscaler asked the CVO to remove the config map [3]. That lead some born-in 4.3 clusters to have crashlooping autoscalers, because the mount attempts kept failing on the missing config map. We couldn't think of a plausible reason why cluster admins would want to inject additional volume mounts in a CVO-managed pod configuration, so this commit removes that ability and begins clearing away any volume(mount) configuration that is not present in the reconciling manifest. Cluster administrators who do need to add additional mounts in an emergency are free to use ClusterVersion's spec.overrides to take control of a particular CVO-managed resource. This joins a series of similar previous tightenings, including 02bb9ba (lib/resourcemerge/core: Clear env and envFrom if unset in manifest, 2021-04-20, openshift#549) and ca299b8 (lib/resourcemerge: remove ports which are no longer required, 2020-02-13, openshift#322). [1]: https://bugzilla.redhat.com/show_bug.cgi?id=2002834 [2]: openshift/cluster-autoscaler-operator@f08589d#diff-547486373183980619528df695869ed32b80c18383bc16b57a5ee931bf0edd39L89 [3]: openshift/cluster-autoscaler-operator@9a7b3be#diff-d0cf785e044c611986a4d9bdd65bb373c86f9eb1c97bd3f105062184342a872dR4
wking
added a commit
to wking/cluster-version-operator
that referenced
this pull request
Sep 14, 2021
Since this package was created in d9f6718 (lib: add lib for applying objects, 2018-08-14, openshift#7), the volume(mount) merge logic has required manifest entries to exist, but has allowed in-cluster entries to persist without removal. That hasn't been a problem until [1]: 1. In 4.3, the autoscaler asked for a ca-cert volume mount, based on the cluster-autoscaler-operator-ca config map. 2. In 4.4, the autoscaler dropped those manifest entries [2]. 3. In 4.9, the autoscaler asked the CVO to remove the config map [3]. That lead some born-in 4.3 clusters to have crashlooping autoscalers, because the mount attempts kept failing on the missing config map. We couldn't think of a plausible reason why cluster admins would want to inject additional volume mounts in a CVO-managed pod configuration, so this commit removes that ability and begins clearing away any volume(mount) configuration that is not present in the reconciling manifest. Cluster administrators who do need to add additional mounts in an emergency are free to use ClusterVersion's spec.overrides to take control of a particular CVO-managed resource. This joins a series of similar previous tightenings, including 02bb9ba (lib/resourcemerge/core: Clear env and envFrom if unset in manifest, 2021-04-20, openshift#549) and ca299b8 (lib/resourcemerge: remove ports which are no longer required, 2020-02-13, openshift#322). [1]: https://bugzilla.redhat.com/show_bug.cgi?id=2002834 [2]: openshift/cluster-autoscaler-operator@f08589d#diff-547486373183980619528df695869ed32b80c18383bc16b57a5ee931bf0edd39L89 [3]: openshift/cluster-autoscaler-operator@9a7b3be#diff-d0cf785e044c611986a4d9bdd65bb373c86f9eb1c97bd3f105062184342a872dR4
wking
added a commit
to wking/cluster-version-operator
that referenced
this pull request
Sep 14, 2021
Since this package was created in d9f6718 (lib: add lib for applying objects, 2018-08-14, openshift#7), the volume(mount) merge logic has required manifest entries to exist, but has allowed in-cluster entries to persist without removal. That hasn't been a problem until [1]: 1. In 4.3, the autoscaler asked for a ca-cert volume mount, based on the cluster-autoscaler-operator-ca config map. 2. In 4.4, the autoscaler dropped those manifest entries [2]. 3. In 4.9, the autoscaler asked the CVO to remove the config map [3]. That lead some born-in 4.3 clusters to have crashlooping autoscalers, because the mount attempts kept failing on the missing config map. We couldn't think of a plausible reason why cluster admins would want to inject additional volume mounts in a CVO-managed pod configuration, so this commit removes that ability and begins clearing away any volume(mount) configuration that is not present in the reconciling manifest. Cluster administrators who do need to add additional mounts in an emergency are free to use ClusterVersion's spec.overrides to take control of a particular CVO-managed resource. This joins a series of similar previous tightenings, including 02bb9ba (lib/resourcemerge/core: Clear env and envFrom if unset in manifest, 2021-04-20, openshift#549) and ca299b8 (lib/resourcemerge: remove ports which are no longer required, 2020-02-13, openshift#322). [1]: https://bugzilla.redhat.com/show_bug.cgi?id=2002834 [2]: openshift/cluster-autoscaler-operator@f08589d#diff-547486373183980619528df695869ed32b80c18383bc16b57a5ee931bf0edd39L89 [3]: openshift/cluster-autoscaler-operator@9a7b3be#diff-d0cf785e044c611986a4d9bdd65bb373c86f9eb1c97bd3f105062184342a872dR4
wking
added a commit
to wking/cluster-version-operator
that referenced
this pull request
Sep 14, 2021
Since this package was created in d9f6718 (lib: add lib for applying objects, 2018-08-14, openshift#7), the volume(mount) merge logic has required manifest entries to exist, but has allowed in-cluster entries to persist without removal. That hasn't been a problem until [1]: 1. In 4.3, the autoscaler asked for a ca-cert volume mount, based on the cluster-autoscaler-operator-ca config map. 2. In 4.4, the autoscaler dropped those manifest entries [2]. 3. In 4.9, the autoscaler asked the CVO to remove the config map [3]. That lead some born-in 4.3 clusters to have crashlooping autoscalers, because the mount attempts kept failing on the missing config map. We couldn't think of a plausible reason why cluster admins would want to inject additional volume mounts in a CVO-managed pod configuration, so this commit removes that ability and begins clearing away any volume(mount) configuration that is not present in the reconciling manifest. Cluster administrators who do need to add additional mounts in an emergency are free to use ClusterVersion's spec.overrides to take control of a particular CVO-managed resource. This joins a series of similar previous tightenings, including 02bb9ba (lib/resourcemerge/core: Clear env and envFrom if unset in manifest, 2021-04-20, openshift#549) and ca299b8 (lib/resourcemerge: remove ports which are no longer required, 2020-02-13, openshift#322). [1]: https://bugzilla.redhat.com/show_bug.cgi?id=2002834 [2]: openshift/cluster-autoscaler-operator@f08589d#diff-547486373183980619528df695869ed32b80c18383bc16b57a5ee931bf0edd39L89 [3]: openshift/cluster-autoscaler-operator@9a7b3be#diff-d0cf785e044c611986a4d9bdd65bb373c86f9eb1c97bd3f105062184342a872dR4
openshift-cherrypick-robot
pushed a commit
to openshift-cherrypick-robot/cluster-version-operator
that referenced
this pull request
Sep 16, 2021
Since this package was created in d9f6718 (lib: add lib for applying objects, 2018-08-14, openshift#7), the volume(mount) merge logic has required manifest entries to exist, but has allowed in-cluster entries to persist without removal. That hasn't been a problem until [1]: 1. In 4.3, the autoscaler asked for a ca-cert volume mount, based on the cluster-autoscaler-operator-ca config map. 2. In 4.4, the autoscaler dropped those manifest entries [2]. 3. In 4.9, the autoscaler asked the CVO to remove the config map [3]. That lead some born-in 4.3 clusters to have crashlooping autoscalers, because the mount attempts kept failing on the missing config map. We couldn't think of a plausible reason why cluster admins would want to inject additional volume mounts in a CVO-managed pod configuration, so this commit removes that ability and begins clearing away any volume(mount) configuration that is not present in the reconciling manifest. Cluster administrators who do need to add additional mounts in an emergency are free to use ClusterVersion's spec.overrides to take control of a particular CVO-managed resource. This joins a series of similar previous tightenings, including 02bb9ba (lib/resourcemerge/core: Clear env and envFrom if unset in manifest, 2021-04-20, openshift#549) and ca299b8 (lib/resourcemerge: remove ports which are no longer required, 2020-02-13, openshift#322). [1]: https://bugzilla.redhat.com/show_bug.cgi?id=2002834 [2]: openshift/cluster-autoscaler-operator@f08589d#diff-547486373183980619528df695869ed32b80c18383bc16b57a5ee931bf0edd39L89 [3]: openshift/cluster-autoscaler-operator@9a7b3be#diff-d0cf785e044c611986a4d9bdd65bb373c86f9eb1c97bd3f105062184342a872dR4
wking
added a commit
to wking/cluster-version-operator
that referenced
this pull request
Jul 25, 2023
This is from 9cf86f3 (cmd: update cmd for informers, 2018-08-14, openshift#7). I'm not completely sure what it was about, but I expect it was a copy/paste error referencing a MachineConfig controller.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uses informer pattern for CVO
/assign @crawford