openshift / cluster-version-operator

Apache License 2.0
84 stars 191 forks source link

OCPBUGS-44328: Fix desired before sync_worker's work is initialized #1104

Closed hongkailiu closed 1 week ago

hongkailiu commented 3 weeks ago

This PR is generated by

$ git cherry-pick a411e9da061c6ba1b71bb2b1ce5972a8e0c03307 6906bbfb9d075450e93680cdf8ccfc51d4e62c24 e3a05d439a609e1eb5fc3c63c70a7825ee48f41f a1ee1ca6c050b752c622264c6903823210671334 ae4e180b19aa4f67f2a53785813ff040ee1690da

and then fix the backported unit test TestCVO_UpgradePayloadStillInitializing by the following editting:

git --no-pager diff 
diff --git a/pkg/cvo/cvo_scenarios_test.go b/pkg/cvo/cvo_scenarios_test.go
index c195e26c..9f060195 100644
--- a/pkg/cvo/cvo_scenarios_test.go
+++ b/pkg/cvo/cvo_scenarios_test.go
@@ -33,9 +33,18 @@ import (
 )

 var architecture string
+var sortedCaps = configv1.ClusterVersionCapabilitySets[configv1.ClusterVersionCapabilitySetCurrent]
+var sortedKnownCaps = configv1.KnownClusterVersionCapabilities

 func init() {
        architecture = runtime.GOARCH
+
+       sort.Slice(sortedCaps, func(i, j int) bool {
+               return sortedCaps[i] < sortedCaps[j]
+       })
+       sort.Slice(sortedKnownCaps, func(i, j int) bool {
+               return sortedKnownCaps[i] < sortedKnownCaps[j]
+       })
 }

 func setupCVOTest(payloadDir string) (*Operator, map[string]apiruntime.Object, *fake.Clientset, *dynamicfake.FakeDynamicClient, func()) {
diff --git a/pkg/cvo/sync_worker.go b/pkg/cvo/sync_worker.go
index 22ec05b9..eb2fb500 100644
--- a/pkg/cvo/sync_worker.go
+++ b/pkg/cvo/sync_worker.go
@@ -191,11 +191,6 @@ type SyncWorker struct {

        clusterProfile string

-       // alwaysEnableCapabilities is a list of cluster capabilities which should
-       // always be implicitly enabled.
-       // This contributes to whether or not some manifests are included for reconciliation.
-       alwaysEnableCapabilities []configv1.ClusterVersionCapability
-
        // initializedFunc is only for the unit-test purpose
        initializedFunc func() bool
 }

where alwaysEnableCapabilities is removed because it is not used in 4.15 and the variables sortedCaps and sortedKnownCaps are added because they are used in the backported unit test.

Note that https://github.com/openshift/cluster-version-operator/pull/1097/commits/e94580f609e346bf8bee71ef2e362bf76cc8b0df is not picked to keep the manual work simple. Otherwise it would lead to more manual work such as fixing using constants from the new API like configv1.ClusterVersionCapabilityIngress which is not available for 4.15.

hongkailiu commented 3 weeks ago

/test-required

hongkailiu commented 3 weeks ago

/test unit

hongkailiu commented 3 weeks ago

/retest-required

openshift-ci[bot] commented 3 weeks ago

@hongkailiu: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
hongkailiu commented 3 weeks ago

/jira cherrypick OCPBUGS-43964

openshift-ci-robot commented 3 weeks ago

@hongkailiu: Jira Issue OCPBUGS-43964 has been cloned as Jira Issue OCPBUGS-44328. Will retitle bug to link to clone. /retitle OCPBUGS-44328: [wip][release-4.15] OCPBUGS-TODO: Fix desired before sync_worker's work is initialized

In response to [this](https://github.com/openshift/cluster-version-operator/pull/1104#issuecomment-2462335069): >/jira cherrypick OCPBUGS-43964 Instructions for interacting with me using PR comments are available [here](https://prow.ci.openshift.org/command-help?repo=openshift%2Fcluster-version-operator). If you have questions or suggestions related to my behavior, please file an issue against the [openshift-eng/jira-lifecycle-plugin](https://github.com/openshift-eng/jira-lifecycle-plugin/issues/new) repository.
openshift-ci-robot commented 3 weeks ago

@hongkailiu: This pull request references Jira Issue OCPBUGS-44328, which is valid. The bug has been moved to the POST state.

7 validation(s) were run on this bug * bug is open, matching expected state (open) * bug target version (4.15.z) matches configured target version for branch (4.15.z) * bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST) * release note text is set and does not match the template * dependent bug [Jira Issue OCPBUGS-43964](https://issues.redhat.com//browse/OCPBUGS-43964) is in the state Verified, which is one of the valid states (VERIFIED, RELEASE PENDING, CLOSED (ERRATA), CLOSED (CURRENT RELEASE), CLOSED (DONE), CLOSED (DONE-ERRATA)) * dependent [Jira Issue OCPBUGS-43964](https://issues.redhat.com//browse/OCPBUGS-43964) targets the "4.16.z" version, which is one of the valid target versions: 4.16.0, 4.16.z * bug has dependents

Requesting review from QA contact: /cc @jiajliu

The bug has been updated to refer to the pull request using the external bug tracker.

In response to [this](https://github.com/openshift/cluster-version-operator/pull/1104): >This PR is generated by > >``` >$ git cherry-pick a411e9da061c6ba1b71bb2b1ce5972a8e0c03307 6906bbfb9d075450e93680cdf8ccfc51d4e62c24 e3a05d439a609e1eb5fc3c63c70a7825ee48f41f a1ee1ca6c050b752c622264c6903823210671334 ae4e180b19aa4f67f2a53785813ff040ee1690da >``` > >and then fix [the backported unit test](https://github.com/openshift/cluster-version-operator/pull/1097/files#diff-6cca1696ca6d83b25f5baf586b99602d32827c05c4276dd03ebfcb6825ff2558R2865) `TestCVO_UpgradePayloadStillInitializing` by the following editting: > >```console >git --no-pager diff >diff --git a/pkg/cvo/cvo_scenarios_test.go b/pkg/cvo/cvo_scenarios_test.go >index c195e26c..9f060195 100644 >--- a/pkg/cvo/cvo_scenarios_test.go >+++ b/pkg/cvo/cvo_scenarios_test.go >@@ -33,9 +33,18 @@ import ( > ) > > var architecture string >+var sortedCaps = configv1.ClusterVersionCapabilitySets[configv1.ClusterVersionCapabilitySetCurrent] >+var sortedKnownCaps = configv1.KnownClusterVersionCapabilities > > func init() { > architecture = runtime.GOARCH >+ >+ sort.Slice(sortedCaps, func(i, j int) bool { >+ return sortedCaps[i] < sortedCaps[j] >+ }) >+ sort.Slice(sortedKnownCaps, func(i, j int) bool { >+ return sortedKnownCaps[i] < sortedKnownCaps[j] >+ }) > } > > func setupCVOTest(payloadDir string) (*Operator, map[string]apiruntime.Object, *fake.Clientset, *dynamicfake.FakeDynamicClient, func()) { >diff --git a/pkg/cvo/sync_worker.go b/pkg/cvo/sync_worker.go >index 22ec05b9..eb2fb500 100644 >--- a/pkg/cvo/sync_worker.go >+++ b/pkg/cvo/sync_worker.go >@@ -191,11 +191,6 @@ type SyncWorker struct { > > clusterProfile string > >- // alwaysEnableCapabilities is a list of cluster capabilities which should >- // always be implicitly enabled. >- // This contributes to whether or not some manifests are included for reconciliation. >- alwaysEnableCapabilities []configv1.ClusterVersionCapability >- > // initializedFunc is only for the unit-test purpose > initializedFunc func() bool > } > >``` > >where `alwaysEnableCapabilities` is removed because it is not used in 4.15 and the variables `sortedCaps` and `sortedKnownCaps` are added because they are [used](https://github.com/openshift/cluster-version-operator/pull/1097/files#diff-6cca1696ca6d83b25f5baf586b99602d32827c05c4276dd03ebfcb6825ff2558R2946-R2947) in the backported unit test. > >Note that https://github.com/openshift/cluster-version-operator/pull/1097/commits/e94580f609e346bf8bee71ef2e362bf76cc8b0df is not picked to keep the manual work simple. Otherwise it would lead to more manual work such as fixing using constants from the new API like `configv1.ClusterVersionCapabilityIngress`. > > > Instructions for interacting with me using PR comments are available [here](https://prow.ci.openshift.org/command-help?repo=openshift%2Fcluster-version-operator). If you have questions or suggestions related to my behavior, please file an issue against the [openshift-eng/jira-lifecycle-plugin](https://github.com/openshift-eng/jira-lifecycle-plugin/issues/new) repository.
hongkailiu commented 3 weeks ago

/jira refresh

openshift-ci-robot commented 3 weeks ago

@hongkailiu: This pull request references Jira Issue OCPBUGS-44328, which is valid.

7 validation(s) were run on this bug * bug is open, matching expected state (open) * bug target version (4.15.z) matches configured target version for branch (4.15.z) * bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST) * release note text is set and does not match the template * dependent bug [Jira Issue OCPBUGS-43964](https://issues.redhat.com//browse/OCPBUGS-43964) is in the state Verified, which is one of the valid states (VERIFIED, RELEASE PENDING, CLOSED (ERRATA), CLOSED (CURRENT RELEASE), CLOSED (DONE), CLOSED (DONE-ERRATA)) * dependent [Jira Issue OCPBUGS-43964](https://issues.redhat.com//browse/OCPBUGS-43964) targets the "4.16.z" version, which is one of the valid target versions: 4.16.0, 4.16.z * bug has dependents

Requesting review from QA contact: /cc @jiajliu

The bug has been updated to refer to the pull request using the external bug tracker.

In response to [this](https://github.com/openshift/cluster-version-operator/pull/1104#issuecomment-2462342547): >/jira refresh Instructions for interacting with me using PR comments are available [here](https://prow.ci.openshift.org/command-help?repo=openshift%2Fcluster-version-operator). If you have questions or suggestions related to my behavior, please file an issue against the [openshift-eng/jira-lifecycle-plugin](https://github.com/openshift-eng/jira-lifecycle-plugin/issues/new) repository.
wking commented 2 weeks ago

/retitle OCPBUGS-44328: Fix desired before sync_worker's work is initialized

openshift-ci[bot] commented 2 weeks ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hongkailiu, wking

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: - ~~[OWNERS](https://github.com/openshift/cluster-version-operator/blob/release-4.15/OWNERS)~~ [wking] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
evakhoni commented 2 weeks ago

so the only change outside of the unit test, is the alwaysEnableCapabilities in sync worker, which anyway weren't there in 4.15 right? if such, no real change in behavior compared to the parent fix right?

hongkailiu commented 2 weeks ago

so the only change outside of the unit test, is the alwaysEnableCapabilities in sync worker, which anyway weren't there in 4.15 right? if such, no real change in behavior compared to the parent fix right?

Correct and correct.

evakhoni commented 1 week ago

ok then, verified. /label qe-approved /cc @jiajliu for approval tnx

openshift-ci-robot commented 1 week ago

@hongkailiu: This pull request references Jira Issue OCPBUGS-44328, which is valid.

7 validation(s) were run on this bug * bug is open, matching expected state (open) * bug target version (4.15.z) matches configured target version for branch (4.15.z) * bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST) * release note text is set and does not match the template * dependent bug [Jira Issue OCPBUGS-43964](https://issues.redhat.com//browse/OCPBUGS-43964) is in the state Closed (Done-Errata), which is one of the valid states (VERIFIED, RELEASE PENDING, CLOSED (ERRATA), CLOSED (CURRENT RELEASE), CLOSED (DONE), CLOSED (DONE-ERRATA)) * dependent [Jira Issue OCPBUGS-43964](https://issues.redhat.com//browse/OCPBUGS-43964) targets the "4.16.z" version, which is one of the valid target versions: 4.16.0, 4.16.z * bug has dependents

Requesting review from QA contact: /cc @evakhoni

The bug has been updated to refer to the pull request using the external bug tracker.

In response to [this](https://github.com/openshift/cluster-version-operator/pull/1104): >This PR is generated by > >``` >$ git cherry-pick a411e9da061c6ba1b71bb2b1ce5972a8e0c03307 6906bbfb9d075450e93680cdf8ccfc51d4e62c24 e3a05d439a609e1eb5fc3c63c70a7825ee48f41f a1ee1ca6c050b752c622264c6903823210671334 ae4e180b19aa4f67f2a53785813ff040ee1690da >``` > >and then fix [the backported unit test](https://github.com/openshift/cluster-version-operator/pull/1097/files#diff-6cca1696ca6d83b25f5baf586b99602d32827c05c4276dd03ebfcb6825ff2558R2865) `TestCVO_UpgradePayloadStillInitializing` by the following editting: > >```console >git --no-pager diff >diff --git a/pkg/cvo/cvo_scenarios_test.go b/pkg/cvo/cvo_scenarios_test.go >index c195e26c..9f060195 100644 >--- a/pkg/cvo/cvo_scenarios_test.go >+++ b/pkg/cvo/cvo_scenarios_test.go >@@ -33,9 +33,18 @@ import ( > ) > > var architecture string >+var sortedCaps = configv1.ClusterVersionCapabilitySets[configv1.ClusterVersionCapabilitySetCurrent] >+var sortedKnownCaps = configv1.KnownClusterVersionCapabilities > > func init() { > architecture = runtime.GOARCH >+ >+ sort.Slice(sortedCaps, func(i, j int) bool { >+ return sortedCaps[i] < sortedCaps[j] >+ }) >+ sort.Slice(sortedKnownCaps, func(i, j int) bool { >+ return sortedKnownCaps[i] < sortedKnownCaps[j] >+ }) > } > > func setupCVOTest(payloadDir string) (*Operator, map[string]apiruntime.Object, *fake.Clientset, *dynamicfake.FakeDynamicClient, func()) { >diff --git a/pkg/cvo/sync_worker.go b/pkg/cvo/sync_worker.go >index 22ec05b9..eb2fb500 100644 >--- a/pkg/cvo/sync_worker.go >+++ b/pkg/cvo/sync_worker.go >@@ -191,11 +191,6 @@ type SyncWorker struct { > > clusterProfile string > >- // alwaysEnableCapabilities is a list of cluster capabilities which should >- // always be implicitly enabled. >- // This contributes to whether or not some manifests are included for reconciliation. >- alwaysEnableCapabilities []configv1.ClusterVersionCapability >- > // initializedFunc is only for the unit-test purpose > initializedFunc func() bool > } > >``` > >where `alwaysEnableCapabilities` is removed because it is not used in 4.15 and the variables `sortedCaps` and `sortedKnownCaps` are added because they are [used](https://github.com/openshift/cluster-version-operator/pull/1097/files#diff-6cca1696ca6d83b25f5baf586b99602d32827c05c4276dd03ebfcb6825ff2558R2946-R2947) in the backported unit test. > >Note that https://github.com/openshift/cluster-version-operator/pull/1097/commits/e94580f609e346bf8bee71ef2e362bf76cc8b0df is not picked to keep the manual work simple. Otherwise it would lead to more manual work such as fixing using constants from the new API like `configv1.ClusterVersionCapabilityIngress` which is not available for 4.15. > > > Instructions for interacting with me using PR comments are available [here](https://prow.ci.openshift.org/command-help?repo=openshift%2Fcluster-version-operator). If you have questions or suggestions related to my behavior, please file an issue against the [openshift-eng/jira-lifecycle-plugin](https://github.com/openshift-eng/jira-lifecycle-plugin/issues/new) repository.
hongkailiu commented 1 week ago

/cherrypick release-4.14

openshift-cherrypick-robot commented 1 week ago

@hongkailiu: once the present PR merges, I will cherry-pick it on top of release-4.14 in a new PR and assign it to you.

In response to [this](https://github.com/openshift/cluster-version-operator/pull/1104#issuecomment-2483461960): >/cherrypick release-4.14 Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
jiajliu commented 1 week ago

/label cherry-pick-approved

openshift-ci-robot commented 1 week ago

@hongkailiu: Jira Issue OCPBUGS-44328: All pull requests linked via external trackers have merged:

Jira Issue OCPBUGS-44328 has been moved to the MODIFIED state.

In response to [this](https://github.com/openshift/cluster-version-operator/pull/1104): >This PR is generated by > >``` >$ git cherry-pick a411e9da061c6ba1b71bb2b1ce5972a8e0c03307 6906bbfb9d075450e93680cdf8ccfc51d4e62c24 e3a05d439a609e1eb5fc3c63c70a7825ee48f41f a1ee1ca6c050b752c622264c6903823210671334 ae4e180b19aa4f67f2a53785813ff040ee1690da >``` > >and then fix [the backported unit test](https://github.com/openshift/cluster-version-operator/pull/1097/files#diff-6cca1696ca6d83b25f5baf586b99602d32827c05c4276dd03ebfcb6825ff2558R2865) `TestCVO_UpgradePayloadStillInitializing` by the following editting: > >```console >git --no-pager diff >diff --git a/pkg/cvo/cvo_scenarios_test.go b/pkg/cvo/cvo_scenarios_test.go >index c195e26c..9f060195 100644 >--- a/pkg/cvo/cvo_scenarios_test.go >+++ b/pkg/cvo/cvo_scenarios_test.go >@@ -33,9 +33,18 @@ import ( > ) > > var architecture string >+var sortedCaps = configv1.ClusterVersionCapabilitySets[configv1.ClusterVersionCapabilitySetCurrent] >+var sortedKnownCaps = configv1.KnownClusterVersionCapabilities > > func init() { > architecture = runtime.GOARCH >+ >+ sort.Slice(sortedCaps, func(i, j int) bool { >+ return sortedCaps[i] < sortedCaps[j] >+ }) >+ sort.Slice(sortedKnownCaps, func(i, j int) bool { >+ return sortedKnownCaps[i] < sortedKnownCaps[j] >+ }) > } > > func setupCVOTest(payloadDir string) (*Operator, map[string]apiruntime.Object, *fake.Clientset, *dynamicfake.FakeDynamicClient, func()) { >diff --git a/pkg/cvo/sync_worker.go b/pkg/cvo/sync_worker.go >index 22ec05b9..eb2fb500 100644 >--- a/pkg/cvo/sync_worker.go >+++ b/pkg/cvo/sync_worker.go >@@ -191,11 +191,6 @@ type SyncWorker struct { > > clusterProfile string > >- // alwaysEnableCapabilities is a list of cluster capabilities which should >- // always be implicitly enabled. >- // This contributes to whether or not some manifests are included for reconciliation. >- alwaysEnableCapabilities []configv1.ClusterVersionCapability >- > // initializedFunc is only for the unit-test purpose > initializedFunc func() bool > } > >``` > >where `alwaysEnableCapabilities` is removed because it is not used in 4.15 and the variables `sortedCaps` and `sortedKnownCaps` are added because they are [used](https://github.com/openshift/cluster-version-operator/pull/1097/files#diff-6cca1696ca6d83b25f5baf586b99602d32827c05c4276dd03ebfcb6825ff2558R2946-R2947) in the backported unit test. > >Note that https://github.com/openshift/cluster-version-operator/pull/1097/commits/e94580f609e346bf8bee71ef2e362bf76cc8b0df is not picked to keep the manual work simple. Otherwise it would lead to more manual work such as fixing using constants from the new API like `configv1.ClusterVersionCapabilityIngress` which is not available for 4.15. > > > Instructions for interacting with me using PR comments are available [here](https://prow.ci.openshift.org/command-help?repo=openshift%2Fcluster-version-operator). If you have questions or suggestions related to my behavior, please file an issue against the [openshift-eng/jira-lifecycle-plugin](https://github.com/openshift-eng/jira-lifecycle-plugin/issues/new) repository.
openshift-cherrypick-robot commented 1 week ago

@hongkailiu: new pull request created: #1109

In response to [this](https://github.com/openshift/cluster-version-operator/pull/1104#issuecomment-2483461960): >/cherrypick release-4.14 Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
openshift-bot commented 1 week ago

[ART PR BUILD NOTIFIER]

Distgit: cluster-version-operator This PR has been included in build cluster-version-operator-container-v4.15.0-202411190104.p0.gfbb41e8.assembly.stream.el9. All builds following this will include this PR.

openshift-merge-robot commented 1 week ago

Fix included in accepted release 4.15.0-0.nightly-2024-11-19-015320