WIP: Add baremetalds-e2e-upgrade-workflow #12779

eisraeli · 2020-10-13T22:40:42Z

https://issues.redhat.com/browse/KNIDEPLOY-3733

Signed-off-by: Eran Israeli eisraeli@redhat.com

openshift-ci-robot · 2020-10-13T22:40:58Z

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

eisraeli · 2020-10-13T23:23:00Z

/assign @andfasano @vrutkovs

Signed-off-by: Eran Israeli <eisraeli@redhat.com>

openshift-ci-robot · 2020-10-14T07:26:35Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: eisraeli
To complete the pull request process, please assign andfasano after the PR has been reviewed.
You can assign the PR to them by writing /assign @andfasano in a comment when ready.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

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

andfasano · 2020-10-14T07:38:05Z

ci-operator/step-registry/baremetalds/e2e/upgrade/baremetalds-e2e-upgrade-workflow.yaml

+    test:
+      - ref: baremetalds-e2e-conf
+      - ref: baremetalds-e2e-test
+      - ref: baremetalds-e2e-upgrade-upgrade


Splitting the upgrade step from the test step will cause an unnecessary tests execution during the first e2e-test step (which is something already exercised in the standard workflow), thus increasing also the overall time for the job. It would be preferred to merge the two steps together with a conditional execution (in a way similar to the other profiles) here:

release/ci-operator/step-registry/baremetalds/e2e/test/baremetalds-e2e-test-commands.sh

Line 41 in 8cb0233

ssh "${SSHOPTS[@]}" "root@${IP}" openshift-tests run "openshift/conformance/parallel" --dry-run \| grep -Ff /tmp/test-list \|openshift-tests run -o /tmp/artifacts/e2e.log --junit-dir /tmp/artifacts/junit -f -

andfasano · 2020-10-14T07:41:47Z

ci-operator/step-registry/baremetalds/e2e/upgrade/baremetalds-e2e-upgrade-workflow.yaml

+      OPENSHIFT_UPGRADE_RELEASE_IMAGE_OVERRIDE: "release:latest"   
+    env:
+      DEVSCRIPTS_CONFIG: |
+        OPENSHIFT_RELEASE_IMAGE=$OPENSHIFT_INSTALL_RELEASE_IMAGE_OVERRIDE


I don't think this kind of indirection is supported. In any case it should not be necessary to set DEVSCRIPTS_CONFIG but I think it would be sufficient something like dependencies: RELEASE_IMAGE_LATEST: "release:initial" (see

release/ci-operator/step-registry/baremetalds/devscripts/setup/baremetalds-devscripts-setup-commands.sh

Line 94 in 8cb0233

echo "export OPENSHIFT_RELEASE_IMAGE=${RELEASE_IMAGE_LATEST}" >> /root/dev-scripts/config_root.sh

)

andfasano · 2020-10-14T07:45:41Z

...or/step-registry/baremetalds/e2e/upgrade/upgrade/baremetalds-e2e-upgrade-upgrade-commands.sh

+set -o errexit
+set -o pipefail
+
+openshift-tests run-upgrade --to-image "${OPENSHIFT_UPGRADE_RELEASE_IMAGE_OVERRIDE}" --dry-run all


This unfortunately cannot work, since the openshift-tests binary must be copied on the remote Packet instance where the cluster is currently running. In addition, tests results must be copied back so that they could be published in the job dashboard by the CI.
The baremetalds-e2e-test it's already taking care of all these task, it would be sufficient to make the openshift-tests command here (

release/ci-operator/step-registry/baremetalds/e2e/test/baremetalds-e2e-test-commands.sh

Line 41 in 8cb0233

ssh "${SSHOPTS[@]}" "root@${IP}" openshift-tests run "openshift/conformance/parallel" --dry-run \| grep -Ff /tmp/test-list \|openshift-tests run -o /tmp/artifacts/e2e.log --junit-dir /tmp/artifacts/junit -f -

) conditional

@andfasano From what I've seen, the openshift-tests binary is copied to the remote Packet instance on the baremetal-e2e-test step.

See:

release/ci-operator/step-registry/baremetalds/e2e/test/baremetalds-e2e-test-commands.sh

Line 33 in 8cb0233

scp "${SSHOPTS[@]}" /usr/bin/openshift-tests /usr/bin/kubectl "root@${IP}:/usr/local/bin"

So therefore, the openshift-tests binary is available for the upgrade step is the test step is executed beforehand.
We can either merge those 2 steps (as you've suggested) or make sure the upgrade step handles copies the 'openshift-tests` binary itself.

Perhaps we could mimic vSphere approach here. There is a permanent cluster running on vSphere, which reuses the platform to create more clusters. In that case ssh/scp is not necessary as permanent cluster can reach the new one directly via KUBECONFIG.
WDYT?

I'd prefer to merge it within the existing and already working baremetalds-e2e-test step. Changes we'll be really minimal, and the step it's already taking care of all the rest. In addition, this will let us quickly go on and focus on the openshift-tests upgrade tests execution itself, which could present some additional difficulties (hopefully not) for the baremetal platform.

openshift-ci-robot · 2020-10-14T07:50:13Z

@eisraeli: The following tests failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/rehearse/openshift-metal3/dev-scripts/master/e2e-metal-ipi-upgrade	`e44f5e7`	link	`/test pj-rehearse`
ci/prow/pj-rehearse	`e44f5e7`	link	`/test pj-rehearse`

Full PR test history. Your PR dashboard.

Details

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.

gbenhaim · 2020-10-25T13:47:40Z

I couldn't update @eisraeli PR so I've created a new one #13120

openshift-merge-robot · 2020-10-30T20:04:59Z

@eisraeli: The following tests failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/release-config	`e44f5e7`	link	`/test release-config`
ci/prow/boskos-config-generation	`e44f5e7`	link	`/test boskos-config-generation`
ci/prow/secret-generator-config-valid	`e44f5e7`	link	`/test secret-generator-config-valid`

Full PR test history. Your PR dashboard.

Details

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-ci-robot · 2020-11-01T10:38:37Z

@eisraeli: PR needs rebase.

Details

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.

stbenjam

This is going to be IPv4-only, right? Is it possible for us to also create an OVN variant to ensure IPv4 OVN upgrades are tested?

stbenjam · 2020-11-04T15:25:09Z

It looks like #13120 supersedes this. Let's keep just one PR open at a time...

/close

openshift-ci-robot · 2020-11-04T15:25:27Z

@stbenjam: Closed this PR.

Details

In response to this:

It looks like #13120 supersedes this. Let's keep just one PR open at a time...

/close

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-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 13, 2020

openshift-ci-robot requested review from hardys and markmc October 13, 2020 22:40

openshift-ci-robot assigned andfasano and vrutkovs Oct 13, 2020

eisraeli mentioned this pull request Oct 13, 2020

WIP: Add baremetalds-e2e-upgrade-workflow #11436

Closed

WIP: Add baremetalds-e2e-upgrade-workflow

e44f5e7

Signed-off-by: Eran Israeli <eisraeli@redhat.com>

andfasano reviewed Oct 14, 2020

View reviewed changes

andfasano mentioned this pull request Oct 22, 2020

ci-operator/step-registry: Unify OPENSHIFT_*_RELEASE_IMAGE dependencies #12959

Closed

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

stbenjam reviewed Nov 4, 2020

View reviewed changes

openshift-ci-robot closed this Nov 4, 2020

WIP: Add baremetalds-e2e-upgrade-workflow #12779

WIP: Add baremetalds-e2e-upgrade-workflow #12779

Uh oh!

Conversation

eisraeli commented Oct 13, 2020

Uh oh!

openshift-ci-robot commented Oct 13, 2020

Uh oh!

eisraeli commented Oct 13, 2020

Uh oh!

openshift-ci-robot commented Oct 14, 2020

Uh oh!

andfasano Oct 14, 2020

Choose a reason for hiding this comment

Uh oh!

andfasano Oct 14, 2020

Choose a reason for hiding this comment

Uh oh!

andfasano Oct 14, 2020

Choose a reason for hiding this comment

Uh oh!

eisraeli Oct 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vrutkovs Oct 14, 2020

Choose a reason for hiding this comment

Uh oh!

andfasano Oct 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

openshift-ci-robot commented Oct 14, 2020

Uh oh!

gbenhaim commented Oct 25, 2020

Uh oh!

openshift-merge-robot commented Oct 30, 2020

Uh oh!

openshift-ci-robot commented Nov 1, 2020

Uh oh!

stbenjam left a comment

Choose a reason for hiding this comment

Uh oh!

stbenjam commented Nov 4, 2020

Uh oh!

openshift-ci-robot commented Nov 4, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

eisraeli Oct 14, 2020 •

edited

Loading

andfasano Oct 14, 2020 •

edited

Loading