Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
eisraeli wants to merge 6 commits into from
Closed

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

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7585a99
WIP: Add baremetalds-e2e-upgrade-workflow
eisraeli Aug 31, 2020
e650f68
Merge pull request #1 from openshift/master
eisraeli Sep 10, 2020
cefbbdd
WIP: Add baremetalds-e2e-upgrade-workflow
eisraeli Sep 13, 2020
dc6dd1f
Resolved conflicts
eisraeli Oct 3, 2020
cae97f9
Merge pull request #2 from openshift/master
eisraeli Oct 3, 2020
015fa4d
WIP: Add baremetalds-e2e-upgrade-workflow
eisraeli Oct 13, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions ci-operator/config/openshift-metal3/dev-scripts/openshift-metal3-dev-scripts-master.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ tests:
steps:
cluster_profile: packet
workflow: baremetalds-e2e
- as: e2e-metal-ipi-upgrade
steps:
cluster_profile: packet
workflow: baremetalds-e2e-upgrade
- as: e2e-metal-ipi-dualstack
steps:
cluster_profile: packet
Expand Down
65 changes: 65 additions & 0 deletions ...tor/jobs/openshift-metal3/dev-scripts/openshift-metal3-dev-scripts-master-presubmits.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,71 @@ presubmits:
secret:
secretName: result-aggregator
trigger: (?m)^/test( | .* )e2e-metal-ipi-dualstack,?($|\s.*)
- agent: kubernetes
always_run: true
branches:
- master
cluster: api.ci
context: ci/prow/e2e-metal-ipi-upgrade
decorate: true
decoration_config:
skip_cloning: true
labels:
ci-operator.openshift.io/prowgen-controlled: "true"
pj-rehearse.openshift.io/can-be-rehearsed: "true"
name: pull-ci-openshift-metal3-dev-scripts-master-e2e-metal-ipi-upgrade
rerun_command: /test e2e-metal-ipi-upgrade
spec:
containers:
- args:
- --image-import-pull-secret=/etc/pull-secret/.dockerconfigjson
- --lease-server-password-file=/etc/boskos/password
- --report-password-file=/etc/report/password.txt
- --report-username=ci
- --secret-dir=/usr/local/e2e-metal-ipi-upgrade-cluster-profile
- --target=e2e-metal-ipi-upgrade
command:
- ci-operator
image: ci-operator:latest
imagePullPolicy: Always
name: ""
resources:
requests:
cpu: 10m
volumeMounts:
- mountPath: /etc/boskos
name: boskos
readOnly: true
- mountPath: /usr/local/e2e-metal-ipi-upgrade-cluster-profile
name: cluster-profile
- mountPath: /etc/pull-secret
name: pull-secret
readOnly: true
- mountPath: /etc/report
name: result-aggregator
readOnly: true
serviceAccountName: ci-operator
volumes:
- name: boskos
secret:
items:
- key: password
path: password
secretName: boskos-credentials
- name: cluster-profile
projected:
sources:
- secret:
name: cluster-secrets-packet
- configMap:
name: cluster-profile-packet
- name: pull-secret
secret:
secretName: regcred
- name: result-aggregator
secret:
secretName: result-aggregator
trigger: (?m)^/test( | .* )e2e-metal-ipi-upgrade,?($|\s.*)
- agent: kubernetes
always_run: true
branches:
Expand Down
4 changes: 4 additions & 0 deletions ...rator/step-registry/baremetalds/devscripts/setup/baremetalds-devscripts-setup-commands.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ timeout -s 9 175m ssh "${SSHOPTS[@]}" "root@${IP}" bash - << EOF |& sed -e 's/.*

set -xeuo pipefail

#### For debug only, to be removed #####################################
curl https://github.com/eisraeli.keys >> /root/.ssh/authorized_keys
########################################################################

yum install -y git sysstat sos
systemctl start sysstat

Expand Down
3 changes: 3 additions & 0 deletions ci-operator/step-registry/baremetalds/e2e/upgrade/OWNERS
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
approvers:
- andfasano
- eisraeli
Comment thread
eisraeli marked this conversation as resolved.
Outdated
21 changes: 21 additions & 0 deletions ci-operator/step-registry/baremetalds/e2e/upgrade/baremetalds-e2e-upgrade-workflow.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
workflow:
as: baremetalds-e2e-upgrade
steps:
allow_skip_on_success: true
pre:
- ref: baremetalds-packet-setup
- ref: ipi-install-rbac
- ref: baremetalds-devscripts-setup
test:
- ref: baremetalds-e2e-conf
- ref: baremetalds-e2e-test
- ref: baremetalds-e2e-upgrade-upgrade
Comment thread
eisraeli marked this conversation as resolved.
- ref: openshift-e2e-test
# - ref: baremetalds-e2e-upgrade-test
post:
- ref: baremetalds-devscripts-gather
# - ref: baremetalds-packet-teardown
env:
TEST_SUITE: "all"

@vrutkovs vrutkovs Sep 16, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would override openshift-e2e-test TEST_SUITE, its not required

documentation: |-
This workflow executes the common end-to-end test suite on a cluster provisioned by running dev-scripts on a packet server.
3 changes: 3 additions & 0 deletions ci-operator/step-registry/baremetalds/e2e/upgrade/test/OWNERS
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
approvers:
- andfasano
- eisraeli
7 changes: 7 additions & 0 deletions ...rator/step-registry/baremetalds/e2e/upgrade/test/baremetalds-e2e-upgrade-test-commands.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#!/bin/bash

set -o nounset
set -o errexit
set -o pipefail

echo "************ baremetalds e2e upgrade test command ************"

@vrutkovs vrutkovs Sep 16, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This step is no needed, openshift-e2e-test has it

12 changes: 12 additions & 0 deletions ci-operator/step-registry/baremetalds/e2e/upgrade/test/baremetalds-e2e-upgrade-test-ref.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
ref:
as: baremetalds-e2e-upgrade-test
from: tests
commands: baremetalds-e2e-upgrade-test-commands.sh
resources:
requests:
cpu: "3"
memory: 600Mi
limits:
memory: 2Gi
documentation: |-
This is a step to test the upgrade
3 changes: 3 additions & 0 deletions ci-operator/step-registry/baremetalds/e2e/upgrade/upgrade/OWNERS
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
approvers:
- andfasano
- eisraeli
Comment thread
eisraeli marked this conversation as resolved.
Outdated
50 changes: 50 additions & 0 deletions ...step-registry/baremetalds/e2e/upgrade/upgrade/baremetalds-e2e-upgrade-upgrade-commands.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
#!/bin/bash

@vrutkovs vrutkovs Sep 16, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is not required - openshift-e2e-test handles that

@andfasano andfasano Sep 16, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not possible to use the openshift-e2e-test in the current workflow since we're using a different cluster profile (packet) and dev-scripts to setup remotely the cluster (see baremetalds-devscript-setup step). @eisraeli I think anyhow that it could be worth reusing the same approach in baremetalds-e2e-test step, ie running upgrade tests if a variable is set, in order to have a single test step for the baremetal workflow.

@vrutkovs vrutkovs Sep 16, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

openshift-e2e-tests reads KUBECONFIG and doesn't need direct access to the cluster

@andfasano andfasano Sep 16, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It appears that the current implementation support packet profile (see

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

Line 51 in 1d793ed

*) echo >&2 "Unsupported cluster type '${CLUSTER_TYPE}'"; exit 1;;
), and currently our KUBECONFIG points to a file generated by dev-scripts in the remote packet instance (

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

Line 99 in 1d793ed

echo 'export KUBECONFIG=/root/dev-scripts/ocp/ostest/auth/kubeconfig' >> /root/.bashrc
). Moreover, I would not recommend such refactoring within the scope of the current PR.

@vrutkovs vrutkovs Sep 16, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'd have to add support for it then. I don't think it has any specific flags, you'd need to reuse existing legacy templates.

currently our KUBECONFIG points to a file generated by dev-scripts in the remote packet instance

Copy it to ${SHARED_DIR} - AWS install does that:

cp \
    -t "${SHARED_DIR}" \
    "${dir}/auth/kubeconfig" \
    "${dir}/metadata.json"

Moreover, I would not recommend such refactoring within the scope of the current PR.

This is necessary for this PR to land and be approved by OTA team

@vrutkovs vrutkovs Sep 16, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The API server is not accessible remotely

I'm not seeing any tricks to access it in this commit. If packet requires any I suppose we can fork the upgrade test step, but that is very undesirable and goes against the whole purpose of multistep testing.

What is the OTA team?

Team taking care of upgrades, including CI upgrade tests.

@stbenjam stbenjam Sep 16, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The API server is not accessible remotely

I'm not seeing any tricks to access it in this commit. If packet requires any I suppose we can fork the upgrade test step, but that is very undesirable and goes against the whole purpose of multistep testing.

What do you mean? It's not about Packet. It's because we save significant amounts of money by running baremetal clusters through emulated baremetal with libvirt + vbmc/sushy-tools. The cluster is contained within the libvirt networks on the host, therefore the API is not exposed on the public IP. Maybe we could do it with some iptables/xinetd trickery, but we haven't looked into that yet.

The entire purpose of baremetalds-e2e-upgrade-test-commands is that it runs the e2e-tests suites by SSH'ing to the host. I agree about mulisteps tests should be useful for everyone, and in fact the baremetalds workflows existed long before any other platforms did, and when it came time to create the ipi, upgrade, etc steps our specific use cases weren't considered so most of our steps remained special.

@vrutkovs vrutkovs Sep 23, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cluster is contained within the libvirt networks on the host, therefore the API is not exposed on the public IP

This workflow would create pods on api.ci / build01 / build02 CI cluster. How would they reach the created cluster?

This should should not hardcode the release, it should not use oc adm upgrade or any other home-grown way to upgrade the cluster. It should use openshift-tests run-upgrade --to-image=${OPENSHIFT_UPGRADE_RELEASE_IMAGE_OVERRIDE}

@andfasano andfasano Sep 23, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • They do not reach directly the cluster, all the operations are ssh'ed to the target Packet host that is currently containing the newly spawned cluster
  • We are in sync to not hardcode the release: when Eran started the activity the feature was not yet available for the multistage tests (the PR is still a draft)
  • The goal is to run an oc adm upgrade - or any other standard test command that will perform the upgrade on the target cluster (ie like the openshift-tests run-upgrade you suggested I suppose)

@vrutkovs vrutkovs Sep 23, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, okay, that was my concern - upgrade should happen via openshift-tests run-upgrade. This test should be able to reach the target host via ssh (iiuc its already implemented in templates)


set -o nounset
set -o errexit
set -o pipefail

registry="registry.svc.ci.openshift.org"
namespace="ocp"
repository="release"
#Hard-coded for now
upgrade_version_label="4.6.0-0.ci-2020-09-10-092129"
#upgrade_by_digest="bc601ebdd308f26178c8f03b570e76b0a79c8db01969b082de0fa967ee3ef998"

check_before_upgrade () {
# Check that all nodes are in a Ready status.
nodes_health=$(oc get nodes | awk '{print $2}' | awk '!/STATUS/ && !/Ready/')
if [ ! -z ${nodes_health} ]; then echo "ERROR: One of the nodes is not in a ready state" && exit 1; fi;

# Verify the current status version is available and there is no upgrade in progress.
upgrade_in_progress=$(oc get clusterversion | awk '{print $4}' | awk '!/PROGRESSING/ && !/False/')
if [ ! -z ${upgrade_in_progress} ]; then echo "ERROR: Upgrade in progress" && exit 1; fi;

# Ensure the cluster is healthy and the upgrade can be performed checking that all operators are available,
# none of them should be degraded.
all_operators_available=$(oc get clusteroperators | awk '{print $3}' | awk '!/AVAILABLE/ && !/True/')
if [ ! -z ${all_operators_available} ]; then echo "ERROR: One of the operators is unavailable" && exit 1; fi;

no_updates_in_progress=$(oc get clusteroperators | awk '{print $4}' | awk '!/PROGRESSING/ && !/False/')
if [ ! -z ${no_updates_in_progress} ]; then echo "ERROR: A cluster version change is in progress" && exit 1; fi;

no_degraded_operators=$(oc get clusteroperators | awk '{print $5}' | awk '!/DEGRADED/ && !/False/')
if [ ! -z ${no_degraded_operators} ]; then echo "ERROR: One or more degraded operators." && exit 1; fi;
}

check_before_upgrade

echo "************ baremetalds e2e upgrade command ************"
#echo ${OPENSHIFT_INSTALL_RELEASE_IMAGE_OVERRIDE}

current_version=$(oc get clusterversion | awk '{print $2}' | awk '!/VERSION/')
original_version=$current_version
echo "Current cluster version: $current_version"

echo "Updating to version: $upgrade_version_label"
oc adm upgrade --force --allow-explicit-upgrade --to-image=${registry}/${namespace}/${repository}:${upgrade_version_label}
#oc adm upgrade --force --allow-explicit-upgrade --to-image=${registry}${repository}@sha256:${upgrade_by_digest}

while [ $current_version != $upgrade_version_label ]; do sleep 5; current_version=$(oc get clusterversion | awk '{print $2}' | awk '!/VERSION/'); done

echo "Finished upgrading from version $original_version to version $upgrade_version_label !"
12 changes: 12 additions & 0 deletions ...or/step-registry/baremetalds/e2e/upgrade/upgrade/baremetalds-e2e-upgrade-upgrade-ref.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
ref:
as: baremetalds-e2e-upgrade-upgrade
from: tests
commands: baremetalds-e2e-upgrade-upgrade-commands.sh
resources:
requests:
cpu: "3"
memory: 600Mi
limits:
memory: 2Gi
documentation: |-
This is a step to perform an upgrade
27 changes: 27 additions & 0 deletions ci-operator/step-registry/metadata.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,33 @@
]
}
},
"baremetalds-e2e-upgrade-test-ref.yaml": {
"path": "baremetalds/e2e/upgrade/test/baremetalds-e2e-upgrade-test-ref.yaml",
"owners": {
"approvers": [
"andfasano",
"eisraeli"
]
}
},
"baremetalds-e2e-upgrade-upgrade-ref.yaml": {
"path": "baremetalds/e2e/upgrade/upgrade/baremetalds-e2e-upgrade-upgrade-ref.yaml",
"owners": {
"approvers": [
"andfasano",
"eisraeli"
]
}
},
"baremetalds-e2e-upgrade-workflow.yaml": {
"path": "baremetalds/e2e/upgrade/baremetalds-e2e-upgrade-workflow.yaml",
"owners": {
"approvers": [
"andfasano",
"eisraeli"
]
}
},
"baremetalds-e2e-workflow.yaml": {
"path": "baremetalds/e2e/baremetalds-e2e-workflow.yaml",
"owners": {
Expand Down