Add optional E2E tests for MCO using feature gate during bootstrap #20372

JoelSpeed · 2021-07-19T13:19:05Z

Related to openshift/machine-config-operator#2668, this PR adds a tech preview IPI step and the relevant configuration for AWS (this is the first 4 commits, a subset of #19845) then adds optional presubmits for the MCO so that in the future, they will have some signal as to whether the clusters bootstrap correctly when a feature gate is added to the manifests as part of the install.

This job should fail until 2668 is merged. If the cluster bootstraps correctly and doesn't degrade on install, then the MCO part of the bootstrap process is working as expected.

CC @yuqi-zhang @rphillips @mrunalp This has come from our conversation on Friday

sinnykumari · 2021-07-19T13:37:03Z

Don't have full context but wondering if the workflow and job named more generic like e2e-aws-featuregate. That way we can keep it afterward as well when we are post techpreview.

Does it need openshift/machine-config-operator#2668 in order for this e2e to pass?

JoelSpeed · 2021-07-19T13:42:41Z

Don't have full context but wondering if the workflow and job named more generic like e2e-aws-featuregate

I named it techpreview because it applies the TechPreviewNoUpgrade feature set, the idea is that it creates the cluster in a tech preview state as a customer may do. I'm happy to name it however the team prefers for the PR status check though if you disagree with my naming. I'd prefer to keep the steps and associated parts named as they are as they form part of a wider TechPreviewNoUpgrade testing effort for the release informing jobs

Does it need openshift/machine-config-operator#2668 in order for this e2e to pass?

Yes it will do. As it's optional, we could merge this early and observe that 2668 fixes it, or we can wait til 2668 merges and see it pass here. Not sure which is the preferred ordering.

I have manually tested the step as part of the other PR and it does cause the clusters to fail on MCO degraded, as we should see from the rehearsal jobs here as well.

sinnykumari · 2021-07-19T15:16:03Z

Don't have full context but wondering if the workflow and job named more generic like e2e-aws-featuregate

I named it techpreview because it applies the TechPreviewNoUpgrade feature set, the idea is that it creates the cluster in a tech preview state as a customer may do. I'm happy to name it however the team prefers for the PR status check though if you disagree with my naming. I'd prefer to keep the steps and associated parts named as they are as they form part of a wider TechPreviewNoUpgrade testing effort for the release informing jobs

thanks for the providing the context. Perhaps, we can name the test e2e-aws-csi-migration-techpreviewor something similar which can let us know what this test is all about?

I have manually tested the step as part of the other PR and it does cause the clusters to fail on MCO degraded, as we should see from the rehearsal jobs here as well.

+1, we can merge this and ensure that this test passes on openshift/machine-config-operator#2668

JoelSpeed · 2021-07-19T16:22:44Z

Perhaps, we can name the test e2e-aws-csi-migration-techpreviewor something similar which can let us know what this test is all about?

Let's try and gather some suggestions here. I'd be hesitant to name it with csi-migration as that is a particular feature, where this will change eventually right? As new releases change the feature gates we wouldn't want to update the test name?
Would e2e-aws-techpreview-featuregate or e2e-aws-techpreview-featuregate-bootstrap make it more obvious that we are bootstrapping the cluster with a techpreview featuregate?

kikisdeliveryservice · 2021-07-19T16:54:51Z

Would e2e-aws-techpreview-featuregate or e2e-aws-techpreview-featuregate-bootstrap make it more obvious that we are bootstrapping the cluster with a techpreview featuregate?

what about e2e-aws-ccm-integration or e2e-aws-cloud-provider-featuregate

JoelSpeed · 2021-07-20T10:12:27Z

what about e2e-aws-ccm-integration or e2e-aws-cloud-provider-featuregate

These names tie the test suite to a particular set of features, but these features will evolve over time and I don't think the plan is to remove this once all the CSI or CCM feature work is done (I may have misunderstood there).

In code version of below: https://github.com/openshift/release/pull/20372/files#diff-08f6f5bd2d927a18416780ba27bb50a6e31ffd5b0b90b6054ed64cc042de2d64R6-R15

apiVersion: config.openshift.io/v1
kind: FeatureGate
metadata:
  annotations:
    include.release.openshift.io/self-managed-high-availability: "true"
    include.release.openshift.io/single-node-developer: "true"
    release.openshift.io/create-only: "true"
  name: cluster
spec:
  featureSet: TechPreviewNoUpgrade

This is what we are applying that causes this test to be different to the standard test suite. My hesitancy in calling this CSI or CCM is because as you can see from that, they aren't explicitly called out there. We expect that the features enabled by this FeatureGate are changed release to release, the docs will show this if you look at how this doc differs per different versions

sinnykumari · 2021-07-20T11:57:46Z

I think we are all getting confused here. Goal of renaming e2e test is to keep it meaningful. Any name that justifies what features or set of features this e2e test is all about is fine. For example: suggested name e2e-aws-techpreview-featuregate sounds fine to me.

JoelSpeed · 2021-07-20T12:54:16Z

@sinnykumari Agreed, have updated it to e2e-aws-techpreview-featuregate as per your comment 🙂

sinnykumari · 2021-07-20T13:01:23Z

...s/openshift/machine-config-operator/openshift-machine-config-operator-master-presubmits.yaml

+      ci-operator.openshift.io/prowgen-controlled: "true"
+      pj-rehearse.openshift.io/can-be-rehearsed: "true"
+    name: pull-ci-openshift-machine-config-operator-master-e2e-aws-techpreview-featuregate
+    rerun_command: /test e2e-aws-techpreview-featuregate


We need to add optional: true

Oh, I had done that, perhaps the regenerate killed it, will add back!

sinnykumari · 2021-07-20T13:01:35Z

...nshift/machine-config-operator/openshift-machine-config-operator-release-4.9-presubmits.yaml

+      ci-operator.openshift.io/prowgen-controlled: "true"
+      pj-rehearse.openshift.io/can-be-rehearsed: "true"
+    name: pull-ci-openshift-machine-config-operator-release-4.9-e2e-aws-techpreview-featuregate
+    rerun_command: /test e2e-aws-techpreview-featuregate


We need to add optional: true

sinnykumari

LGTM.

sinnykumari · 2021-07-20T13:33:05Z

umm, test ci-operator-config-metadata failed. Needs fixed, should be mostly ordering or spacing issue

sinnykumari · 2021-07-20T13:33:59Z

Putting hold until ci-operator-config-metadata test is fixed
/hold

sinnykumari · 2021-07-20T13:52:19Z

/hold cancel
/approve

openshift-ci · 2021-07-20T14:58:50Z

@JoelSpeed: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Rerun command
ci/rehearse/openshift/machine-config-operator/release-4.9/e2e-aws-techpreview	4e1e5feb4d60f7554b9469721ed07e0f28cdae7c	link	`/test pj-rehearse`
ci/rehearse/openshift/machine-config-operator/master/e2e-aws-techpreview	4e1e5feb4d60f7554b9469721ed07e0f28cdae7c	link	`/test pj-rehearse`
ci/rehearse/openshift/machine-config-operator/master/e2e-aws-techpreview-featuregate	`db68f3f`	link	`/test pj-rehearse`
ci/rehearse/openshift/machine-config-operator/release-4.9/e2e-aws-techpreview-featuregate	`db68f3f`	link	`/test pj-rehearse`
ci/prow/pj-rehearse	`db68f3f`	link	`/test pj-rehearse`

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.

sinnykumari · 2021-07-20T15:41:22Z

Some of the file changes made here would also require approval from one the owner from https://github.com/openshift/release/blob/master/ci-operator/step-registry/ipi/OWNERS

JoelSpeed · 2021-07-20T15:52:18Z

/assign @deads2k as you are familiar with the TPNU stuff that we are adding to the step-registry already/ have already approved this in the other PR

@sinnykumari could you re LGTM if you're happy to proceed?

sinnykumari · 2021-07-20T16:21:28Z

I thought my /approve earlier would have added lgtm. These bots acts strangely.
/lgtm

deads2k · 2021-07-20T17:54:39Z

I think this does what it says it does

/approve

openshift-ci · 2021-07-20T17:54:53Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, JoelSpeed, sinnykumari

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:

~~ci-operator/config/openshift/machine-config-operator/OWNERS~~ [sinnykumari]
~~ci-operator/jobs/openshift/machine-config-operator/OWNERS~~ [sinnykumari]
~~ci-operator/step-registry/ipi/OWNERS~~ [deads2k]
~~ci-operator/step-registry/openshift/OWNERS~~ [deads2k]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci · 2021-07-20T18:02:32Z

@JoelSpeed: Updated the following 5 configmaps:

ci-operator-4.9-configs configmap in namespace ci at cluster app.ci using the following files:
- key openshift-machine-config-operator-release-4.9.yaml using file ci-operator/config/openshift/machine-config-operator/openshift-machine-config-operator-release-4.9.yaml
job-config-master configmap in namespace ci at cluster app.ci using the following files:
- key openshift-machine-config-operator-master-presubmits.yaml using file ci-operator/jobs/openshift/machine-config-operator/openshift-machine-config-operator-master-presubmits.yaml
job-config-4.9 configmap in namespace ci at cluster app.ci using the following files:
- key openshift-machine-config-operator-release-4.9-presubmits.yaml using file ci-operator/jobs/openshift/machine-config-operator/openshift-machine-config-operator-release-4.9-presubmits.yaml
step-registry configmap in namespace ci at cluster app.ci using the following files:
- key OWNERS using file ci-operator/step-registry/ipi/aws/pre/techpreview/OWNERS
- key ipi-aws-pre-techpreview-chain.metadata.json using file ci-operator/step-registry/ipi/aws/pre/techpreview/ipi-aws-pre-techpreview-chain.metadata.json
- key ipi-aws-pre-techpreview-chain.yaml using file ci-operator/step-registry/ipi/aws/pre/techpreview/ipi-aws-pre-techpreview-chain.yaml
- key OWNERS using file ci-operator/step-registry/ipi/conf/aws/techpreview/OWNERS
- key ipi-conf-aws-techpreview-chain.metadata.json using file ci-operator/step-registry/ipi/conf/aws/techpreview/ipi-conf-aws-techpreview-chain.metadata.json
- key ipi-conf-aws-techpreview-chain.yaml using file ci-operator/step-registry/ipi/conf/aws/techpreview/ipi-conf-aws-techpreview-chain.yaml
- key OWNERS using file ci-operator/step-registry/ipi/conf/techpreview/OWNERS
- key ipi-conf-techpreview-commands.sh using file ci-operator/step-registry/ipi/conf/techpreview/ipi-conf-techpreview-commands.sh
- key ipi-conf-techpreview-ref.metadata.json using file ci-operator/step-registry/ipi/conf/techpreview/ipi-conf-techpreview-ref.metadata.json
- key ipi-conf-techpreview-ref.yaml using file ci-operator/step-registry/ipi/conf/techpreview/ipi-conf-techpreview-ref.yaml
- key OWNERS using file ci-operator/step-registry/openshift/e2e/aws/techpreview/OWNERS
- key openshift-e2e-aws-techpreview-workflow.metadata.json using file ci-operator/step-registry/openshift/e2e/aws/techpreview/openshift-e2e-aws-techpreview-workflow.metadata.json
- key openshift-e2e-aws-techpreview-workflow.yaml using file ci-operator/step-registry/openshift/e2e/aws/techpreview/openshift-e2e-aws-techpreview-workflow.yaml
- key OWNERS using file ci-operator/step-registry/openshift/e2e/aws/techpreview/serial/OWNERS
- key openshift-e2e-aws-techpreview-serial-workflow.metadata.json using file ci-operator/step-registry/openshift/e2e/aws/techpreview/serial/openshift-e2e-aws-techpreview-serial-workflow.metadata.json
- key openshift-e2e-aws-techpreview-serial-workflow.yaml using file ci-operator/step-registry/openshift/e2e/aws/techpreview/serial/openshift-e2e-aws-techpreview-serial-workflow.yaml
ci-operator-master-configs configmap in namespace ci at cluster app.ci using the following files:
- key openshift-machine-config-operator-master.yaml using file ci-operator/config/openshift/machine-config-operator/openshift-machine-config-operator-master.yaml

In response to this:

Related to openshift/machine-config-operator#2668, this PR adds a tech preview IPI step and the relevant configuration for AWS (this is the first 4 commits, a subset of #19845) then adds optional presubmits for the MCO so that in the future, they will have some signal as to whether the clusters bootstrap correctly when a feature gate is added to the manifests as part of the install.

This job should fail until 2668 is merged. If the cluster bootstraps correctly and doesn't degrade on install, then the MCO part of the bootstrap process is working as expected.

CC @yuqi-zhang @rphillips @mrunalp This has come from our conversation on Friday

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.

JoelSpeed added 4 commits July 19, 2021 13:45

Add TechPreview IPI conf step

f103efc

Add TechPreview conf for AWS

df93239

Add TechPreview pre chain for AWS

2cdf4ef

Add TechPreview E2E workflows for AWS

fa70d98

openshift-ci bot requested review from kikisdeliveryservice and sinnykumari July 19, 2021 13:19

JoelSpeed mentioned this pull request Jul 19, 2021

[OCPCLOUD-1209] Run KubeletConfig FeatureGate sync during bootstrap openshift/machine-config-operator#2668

Merged

JoelSpeed force-pushed the mco-tpnu-e2e branch from 4e1e5fe to e70b997 Compare July 20, 2021 12:51

sinnykumari reviewed Jul 20, 2021

View reviewed changes

JoelSpeed force-pushed the mco-tpnu-e2e branch from e70b997 to 48f372e Compare July 20, 2021 13:08

sinnykumari approved these changes Jul 20, 2021

View reviewed changes

openshift-ci bot assigned sinnykumari Jul 20, 2021

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 20, 2021

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 20, 2021

Add optional tech preview bootstrap E2E tests for MCO presubmits

db68f3f

JoelSpeed force-pushed the mco-tpnu-e2e branch from 48f372e to db68f3f Compare July 20, 2021 13:35

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jul 20, 2021

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 20, 2021

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 20, 2021

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 20, 2021

openshift-merge-robot merged commit e2f8c55 into openshift:master Jul 20, 2021

JoelSpeed deleted the mco-tpnu-e2e branch July 21, 2021 07:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add optional E2E tests for MCO using feature gate during bootstrap #20372

Add optional E2E tests for MCO using feature gate during bootstrap #20372

JoelSpeed commented Jul 19, 2021

sinnykumari commented Jul 19, 2021

JoelSpeed commented Jul 19, 2021

sinnykumari commented Jul 19, 2021

JoelSpeed commented Jul 19, 2021

kikisdeliveryservice commented Jul 19, 2021

JoelSpeed commented Jul 20, 2021

sinnykumari commented Jul 20, 2021

JoelSpeed commented Jul 20, 2021

sinnykumari Jul 20, 2021

JoelSpeed Jul 20, 2021

sinnykumari Jul 20, 2021

sinnykumari left a comment

sinnykumari commented Jul 20, 2021

sinnykumari commented Jul 20, 2021

sinnykumari commented Jul 20, 2021

openshift-ci bot commented Jul 20, 2021

sinnykumari commented Jul 20, 2021

JoelSpeed commented Jul 20, 2021

sinnykumari commented Jul 20, 2021

deads2k commented Jul 20, 2021

openshift-ci bot commented Jul 20, 2021

openshift-ci bot commented Jul 20, 2021

Add optional E2E tests for MCO using feature gate during bootstrap #20372

Add optional E2E tests for MCO using feature gate during bootstrap #20372

Conversation

JoelSpeed commented Jul 19, 2021

sinnykumari commented Jul 19, 2021

JoelSpeed commented Jul 19, 2021

sinnykumari commented Jul 19, 2021

JoelSpeed commented Jul 19, 2021

kikisdeliveryservice commented Jul 19, 2021

JoelSpeed commented Jul 20, 2021

sinnykumari commented Jul 20, 2021

JoelSpeed commented Jul 20, 2021

sinnykumari Jul 20, 2021

Choose a reason for hiding this comment

JoelSpeed Jul 20, 2021

Choose a reason for hiding this comment

sinnykumari Jul 20, 2021

Choose a reason for hiding this comment

sinnykumari left a comment

Choose a reason for hiding this comment

sinnykumari commented Jul 20, 2021

sinnykumari commented Jul 20, 2021

sinnykumari commented Jul 20, 2021

openshift-ci bot commented Jul 20, 2021

sinnykumari commented Jul 20, 2021

JoelSpeed commented Jul 20, 2021

sinnykumari commented Jul 20, 2021

deads2k commented Jul 20, 2021

openshift-ci bot commented Jul 20, 2021

openshift-ci bot commented Jul 20, 2021