WINC-818: [test] Fix upgradeable condition test #1283

mansikulkarni96 · 2022-10-19T19:46:15Z

This PR fixes an issue where the upgradeable condition was not being tested in the e2e test suite. This was the case because the OPERATOR_CONDITION_NAME env var is not being set in the CI env when using the OLM upgrade workflow. The recommendation to fix this was to instead get it from the deployment spec as the condition is present once the operator is deployed. The cache client also had to be replaced with the OLM client to get the operator condition using current context.

openshift-ci · 2022-10-19T19:46:23Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

mansikulkarni96 · 2022-10-19T19:58:44Z

/test aws-e2e-upgrade

mansikulkarni96 · 2022-10-19T21:30:51Z

/test aws-e2e-upgrade
cluster install failed

mansikulkarni96 · 2022-10-20T02:04:19Z

/test aws-e2e-upgrade

mansikulkarni96 · 2022-10-26T05:48:33Z

/test aws-e2e-upgrade

mansikulkarni96 · 2022-11-01T04:52:23Z

/test aws-e2e-upgrade

mansikulkarni96 · 2022-11-01T05:21:20Z

/test aws-e2e-upgrade

mansikulkarni96 · 2022-11-01T13:23:30Z

/test aws-e2e-upgrade

mansikulkarni96 · 2022-11-01T14:40:53Z

/cherry-pick release-4.12

openshift-cherrypick-robot · 2022-11-01T14:40:56Z

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

In response to this:

/cherry-pick release-4.12

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

mansikulkarni96 · 2022-11-01T15:29:18Z

/test aws-e2e-upgrade

mansikulkarni96 · 2022-11-01T17:17:45Z

/test aws-e2e-upgrade

jrvaldes · 2022-11-02T14:35:53Z

test/e2e/validation_test.go

+		return err
+	}
+	// Get the operator condition name using the deployment spec
+	ocName := deployment.Spec.Template.Spec.Containers[0].Env[3].Value


Is it safe to get these values by index? Will the operator condition name always be in the third position?

that has been my observation but I think it will be safer to loop through and find the env var by key, will update.

Also the Container[0] -- is it worth looping over containers and filtering for the the desired one by name?

saifshaikh48 · 2022-11-02T15:02:39Z

test/e2e/validation_test.go

+		return err
+	}
+	// Get the operator condition name using the deployment spec
+	ocName := deployment.Spec.Template.Spec.Containers[0].Env[3].Value


Also the Container[0] -- is it worth looping over containers and filtering for the the desired one by name?

saifshaikh48 · 2022-11-02T15:03:03Z

test/e2e/validation_test.go

-		specCheck := condition.Validate(oc.Spec.Conditions, operators.Upgradeable, expected)
-		statusCheck := condition.Validate(oc.Status.Conditions, operators.Upgradeable, expected)
-		return specCheck && statusCheck, nil
+		return condition.Validate(oc.Status.Conditions, operators.Upgradeable, expected), nil


Any reason to move to away from checking both spec and status? I felt that checking both was important since the upgradeable condition changes so often, we should ensure it has settled at the expected state (i.e. spec == status == expected).

good point, I was thinking why it wasn't available in the object. I realised I was using the v1 client, switched to v2 now and I see conditions in spec now. Updating this as well.

openshift-cherrypick-robot · 2022-11-03T13:17:26Z

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

In response to this:

/cherry-pick release-4.10

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

mansikulkarni96 · 2022-11-03T19:58:58Z

/retest-required

mansikulkarni96 · 2022-11-04T00:38:49Z

/retest-required

go get github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/clientset/versioned go mod tidy && go mod vendor

This commit fixes an issue where the upgradeable condition was not being tested in the e2e test suite. This was the case because the OPERATOR_CONDITION_NAME env var is not being set in the CI env when using the OLM upgrade workflow. The recommendation to fix this was to instead get it from the deployment spec as the condition is present once the operator is deployed. The cache client had to be replaced with the OLM client to get the operator condition using current context.

mansikulkarni96 · 2022-11-04T16:52:34Z

/retest-required

saifshaikh48

/lgtm

I think it's worth noting that a side effect of this PR is that the e2e suite will fail if the operator is not deployed through OLM (this shouldn't affect end users though).

mansikulkarni96 · 2022-11-04T17:09:47Z

I think it's worth noting that a side effect of this PR is that the e2e suite will fail if the operator is not deployed through OLM (this shouldn't affect end users though).

I would like to clarify though the test modified through this PR is testable when running the test suite locally as well as CI through the hack script.

mtnbikenc · 2022-11-07T14:34:28Z

/retest required

openshift-ci · 2022-11-07T14:35:06Z

@mtnbikenc: The /retest command does not accept any targets.
The following commands are available to trigger required jobs:

/test aws-e2e-ccm-install
/test aws-e2e-operator
/test aws-e2e-upgrade
/test azure-e2e-operator
/test build
/test ci-index
/test gcp-e2e-operator
/test images
/test lint
/test platform-none-vsphere-e2e-operator
/test unit
/test vsphere-e2e-operator
/test wicd-unit-vsphere

The following commands are available to trigger optional jobs:

/test azure-e2e-ccm-install
/test vsphere-e2e-ccm-install

Use /test all to run the following jobs that were automatically triggered:

pull-ci-openshift-windows-machine-config-operator-master-aws-e2e-ccm-install
pull-ci-openshift-windows-machine-config-operator-master-aws-e2e-operator
pull-ci-openshift-windows-machine-config-operator-master-aws-e2e-upgrade
pull-ci-openshift-windows-machine-config-operator-master-azure-e2e-operator
pull-ci-openshift-windows-machine-config-operator-master-build
pull-ci-openshift-windows-machine-config-operator-master-ci-index
pull-ci-openshift-windows-machine-config-operator-master-gcp-e2e-operator
pull-ci-openshift-windows-machine-config-operator-master-images
pull-ci-openshift-windows-machine-config-operator-master-lint
pull-ci-openshift-windows-machine-config-operator-master-platform-none-vsphere-e2e-operator
pull-ci-openshift-windows-machine-config-operator-master-unit
pull-ci-openshift-windows-machine-config-operator-master-vsphere-e2e-operator
pull-ci-openshift-windows-machine-config-operator-master-wicd-unit-vsphere

In response to this:

/retest required

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

mtnbikenc · 2022-11-07T14:36:03Z

/retest-required

mtnbikenc · 2022-11-07T17:35:50Z

/retest-required

openshift-ci · 2022-11-08T00:12:18Z

@mansikulkarni96: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

openshift-cherrypick-robot · 2022-11-08T00:17:28Z

@mansikulkarni96: #1283 failed to apply on top of branch "release-4.11":

Applying: Vendor dependencies for OLM clientset
.git/rebase-apply/patch:10320: trailing whitespace.
limited by the interface). 
.git/rebase-apply/patch:10410: trailing whitespace.
environment via benchmarks: 
.git/rebase-apply/patch:522: new blank line at EOF.
+
warning: 3 lines add whitespace errors.
Using index info to reconstruct a base tree...
M	go.mod
M	go.sum
M	vendor/modules.txt
Falling back to patching base and 3-way merge...
Auto-merging vendor/modules.txt
CONFLICT (content): Merge conflict in vendor/modules.txt
Auto-merging go.sum
CONFLICT (content): Merge conflict in go.sum
Auto-merging go.mod
CONFLICT (content): Merge conflict in go.mod
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Vendor dependencies for OLM clientset
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick release-4.11

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-cherrypick-robot · 2022-11-08T00:18:05Z

@mansikulkarni96: #1283 failed to apply on top of branch "release-4.10":

Applying: Vendor dependencies for OLM clientset
.git/rebase-apply/patch:10320: trailing whitespace.
limited by the interface). 
.git/rebase-apply/patch:10410: trailing whitespace.
environment via benchmarks: 
.git/rebase-apply/patch:522: new blank line at EOF.
+
warning: 3 lines add whitespace errors.
Using index info to reconstruct a base tree...
M	go.mod
M	go.sum
M	vendor/modules.txt
Falling back to patching base and 3-way merge...
Auto-merging vendor/modules.txt
CONFLICT (content): Merge conflict in vendor/modules.txt
Auto-merging go.sum
CONFLICT (content): Merge conflict in go.sum
Auto-merging go.mod
CONFLICT (content): Merge conflict in go.mod
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Vendor dependencies for OLM clientset
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick release-4.10

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-cherrypick-robot · 2022-11-08T00:18:43Z

@mansikulkarni96: new pull request created: #1317

In response to this:

/cherry-pick release-4.12

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

saifshaikh48 · 2022-11-08T15:31:20Z

@mansikulkarni96 should this be backported to 4.9 as well (since #618 is also in 4.9)?

mansikulkarni96 · 2022-11-08T15:57:41Z

@mansikulkarni96 should this be backported to 4.9 as well (since #618 is also in 4.9)?

yes it should looking into the failed backports as well.

mansikulkarni96 · 2022-11-08T20:03:28Z

/cherry-pick relase-4.11

openshift-cherrypick-robot · 2022-11-08T20:04:07Z

@mansikulkarni96: cannot checkout relase-4.11: error checking out relase-4.11: exit status 1. output: error: pathspec 'relase-4.11' did not match any file(s) known to git

In response to this:

/cherry-pick relase-4.11

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-cherrypick-robot · 2022-11-08T20:05:33Z

@mansikulkarni96: #1283 failed to apply on top of branch "release-4.11":

Applying: Vendor dependencies for OLM clientset
.git/rebase-apply/patch:10320: trailing whitespace.
limited by the interface). 
.git/rebase-apply/patch:10410: trailing whitespace.
environment via benchmarks: 
.git/rebase-apply/patch:522: new blank line at EOF.
+
warning: 3 lines add whitespace errors.
Using index info to reconstruct a base tree...
M	go.mod
M	go.sum
M	vendor/modules.txt
Falling back to patching base and 3-way merge...
Auto-merging vendor/modules.txt
CONFLICT (content): Merge conflict in vendor/modules.txt
Auto-merging go.sum
CONFLICT (content): Merge conflict in go.sum
Auto-merging go.mod
CONFLICT (content): Merge conflict in go.mod
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Vendor dependencies for OLM clientset
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick release-4.11
not seeing issue backporting locally.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 19, 2022

mansikulkarni96 force-pushed the WINC-818 branch from aad9a6a to 333dd14 Compare October 19, 2022 19:53

mansikulkarni96 force-pushed the WINC-818 branch from 333dd14 to 852b108 Compare October 20, 2022 01:15

mansikulkarni96 force-pushed the WINC-818 branch 2 times, most recently from 6f3a7a7 to 1004e4a Compare November 1, 2022 04:51

mansikulkarni96 changed the title ~~test PR~~ WINC-818: [test] Fix upgradeable condition test Nov 1, 2022

mansikulkarni96 force-pushed the WINC-818 branch from 1004e4a to 7ff6ab4 Compare November 1, 2022 05:21

mansikulkarni96 force-pushed the WINC-818 branch from 7ff6ab4 to 8f578f1 Compare November 1, 2022 13:23

mansikulkarni96 force-pushed the WINC-818 branch from 8f578f1 to c62d05e Compare November 1, 2022 15:22

mansikulkarni96 force-pushed the WINC-818 branch 2 times, most recently from a0f9978 to a006c3e Compare November 2, 2022 03:53

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 2, 2022

mansikulkarni96 force-pushed the WINC-818 branch from a006c3e to be90797 Compare November 2, 2022 13:51

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 2, 2022

mansikulkarni96 force-pushed the WINC-818 branch from be90797 to cd56ac7 Compare November 2, 2022 13:54

jrvaldes reviewed Nov 2, 2022

View reviewed changes

saifshaikh48 reviewed Nov 2, 2022

View reviewed changes

mansikulkarni96 force-pushed the WINC-818 branch from 15ff2ec to 6b8c64e Compare November 2, 2022 17:50

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 4, 2022

mansikulkarni96 force-pushed the WINC-818 branch from 7261ec8 to a025a3a Compare November 4, 2022 16:23

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Nov 4, 2022

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 4, 2022

mansikulkarni96 added 2 commits November 4, 2022 12:29

[vendor] Vendor dependencies for OLM clientset

ab7fc69

go get github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/clientset/versioned go mod tidy && go mod vendor

mansikulkarni96 force-pushed the WINC-818 branch from a025a3a to 0ffe33d Compare November 4, 2022 16:30

saifshaikh48 reviewed Nov 4, 2022

View reviewed changes

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 4, 2022

openshift-merge-robot merged commit feaa3c3 into openshift:master Nov 8, 2022

openshift-cherrypick-robot mentioned this pull request Nov 8, 2022

[release-4.12] WINC-818: [test] Fix upgradeable condition test #1317

Merged

mansikulkarni96 mentioned this pull request Nov 8, 2022

[release-4.11] WINC-818: [test] Fix upgradeable condition test #1318

Merged

WINC-818: [test] Fix upgradeable condition test #1283

WINC-818: [test] Fix upgradeable condition test #1283

Conversation

mansikulkarni96 commented Oct 19, 2022 • edited

openshift-ci bot commented Oct 19, 2022

mansikulkarni96 commented Oct 19, 2022

mansikulkarni96 commented Oct 19, 2022

mansikulkarni96 commented Oct 20, 2022

mansikulkarni96 commented Oct 26, 2022

mansikulkarni96 commented Nov 1, 2022

mansikulkarni96 commented Nov 1, 2022

mansikulkarni96 commented Nov 1, 2022

mansikulkarni96 commented Nov 1, 2022

openshift-cherrypick-robot commented Nov 1, 2022

mansikulkarni96 commented Nov 1, 2022

mansikulkarni96 commented Nov 1, 2022

jrvaldes Nov 2, 2022

Choose a reason for hiding this comment

mansikulkarni96 Nov 2, 2022

Choose a reason for hiding this comment

saifshaikh48 Nov 2, 2022

Choose a reason for hiding this comment

saifshaikh48 Nov 2, 2022

Choose a reason for hiding this comment

saifshaikh48 Nov 2, 2022

Choose a reason for hiding this comment

mansikulkarni96 Nov 2, 2022

Choose a reason for hiding this comment

openshift-cherrypick-robot commented Nov 3, 2022

mansikulkarni96 commented Nov 3, 2022

mansikulkarni96 commented Nov 4, 2022

mansikulkarni96 commented Nov 4, 2022

saifshaikh48 left a comment

Choose a reason for hiding this comment

mansikulkarni96 commented Nov 4, 2022

mtnbikenc commented Nov 7, 2022

openshift-ci bot commented Nov 7, 2022

mtnbikenc commented Nov 7, 2022

mtnbikenc commented Nov 7, 2022

openshift-ci bot commented Nov 8, 2022

openshift-cherrypick-robot commented Nov 8, 2022

openshift-cherrypick-robot commented Nov 8, 2022

openshift-cherrypick-robot commented Nov 8, 2022

saifshaikh48 commented Nov 8, 2022

mansikulkarni96 commented Nov 8, 2022

mansikulkarni96 commented Nov 8, 2022

openshift-cherrypick-robot commented Nov 8, 2022

openshift-cherrypick-robot commented Nov 8, 2022

mansikulkarni96 commented Oct 19, 2022 •

edited