-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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.
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
[SPLAT-1074] vSphere add template field and workflow #41996
[SPLAT-1074] vSphere add template field and workflow #41996
Conversation
/pj-rehearse periodic-ci-openshift-openshift-tests-private-release-4.14-amd64-nightly-vsphere-ipi-template-enable-f14 |
738f1a5
to
15bffa2
Compare
/pj-rehearse periodic-ci-openshift-openshift-tests-private-release-4.14-amd64-nightly-vsphere-ipi-template-enable-f14 |
9b3c6ab
to
39865ef
Compare
/pj-rehearse periodic-ci-openshift-openshift-tests-private-release-4.14-amd64-nightly-vsphere-ipi-template-enable-f14 |
@jianlinliu @gpei @jinyunma please help review the pr when you have time. |
...tor/step-registry/ipi/conf/vsphere/enable-template/ipi-conf-vsphere-enable-template-ref.yaml
Outdated
Show resolved
Hide resolved
...registry/ipi/conf/vsphere/enable-template/ipi-conf-vsphere-enable-template-ref.metadata.json
Outdated
Show resolved
Hide resolved
39865ef
to
e704c68
Compare
selected_hw_version_index=$((RANDOM % ${hw_available_versions})) | ||
target_hw_version=${hw_versions[$selected_hw_version_index]} | ||
echo "$(date -u --rfc-3339=seconds) - Selected hardware version ${target_hw_version}" | ||
vm_template=${vm_template}-hw${target_hw_version} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do not need to upload this vm template?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if no target_hw_version is set, the default hw_version is 15.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was talking about I did not see any commands to upload the target vm template.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reuse the step upi-conf-vsphere-ova
to upload target vm template
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, good to know.
Z_VERSION=$(echo ${VERSION} | cut -d'.' -f2) | ||
if [ ${Z_VERSION} -gt 13 ]; then | ||
cat >> "${ENABLE_TEMPLATE}" << EOF | ||
failureDomains: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seem like this change has a big difference with https://github.com/openshift/release/pull/41672/files, does this work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, I have triggered a job and check the installation log. when reuse the exists rhcos, will not download image from remote.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In your previous PR, you mentioned this feature is for zonal IPI install - ipi-conf-vsphere-zones, per your current logic, seem like the following setting is only working for vsphere-connected
, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
previous i saw this field is under failureDomains, I thought it's a zonal field as i saw the doc said the failure domain establishes the relationships between a region and zone. actually it should be a common feature.
now i add a step to update the install-config to support adding template. we can also add it into vsphere-discon workflow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to know.
@@ -0,0 +1,15 @@ | |||
{ | |||
"path": "cucushift/installer/rehearse/vsphere/ipi/template-enable/provision/cucushift-installer-rehearse-vsphere-ipi-template-enable-provision-chain.yaml", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggest to rename template-enable
to template
installer_bin=$(which openshift-install) | ||
ova_url=$("${installer_bin}" coreos print-stream-json | jq -r '.architectures.x86_64.artifacts.vmware.formats.ova.disk.location') | ||
echo "${ova_url}" > "${SHARED_DIR}"/ova_url.txt | ||
vm_template="${ova_url##*/}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest to introduce a ENV var - RHCOS_VM_TEMPLATE
, its default value is empty, so that we can allow users to override a specific image, when RHCOS_VM_TEMPLATE
is empty, but this step is referenced in a workflow, then we can use the above logic to get a default ova vm template path.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code update done, do we also add a workflow for vsphere-discon?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From my understanding, it makes no differences for template
feature in vsphere connected and disconnected segment.
08788fe
to
783b937
Compare
/pj-rehearse periodic-ci-openshift-openshift-tests-private-release-4.14-amd64-nightly-vsphere-ipi-template-f14 |
vm_template=${vm_template}-hw${target_hw_version} | ||
|
||
ENABLE_TEMPLATE="${SHARED_DIR}"/enable_template_content.txt | ||
VERSION=$(echo "${JOB_NAME}" | grep -o -E '4\.[0-9]+') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Above two lines need to be moved out of if
block? otherwise, it has issues when defining env RHCOS_VM_TEMPLATE
explicitly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agree.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw, @jinyunma for vsphere jobs, always using JOB_NAME
to determinate the cluster version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the job is added into upgrade ci, I think it would have some problem.
E.g:
$ JOB_NAME="periodic-ci-openshift-openshift-tests-private-release-4.12-amd64-nightly-4.12-upgrade-from-stable-4.12-aws-ipi-network-mtu-localzone-sdn-p2-f28"
$ VERSION=$(echo "${JOB_NAME}" | grep -o -E '4\.[0-9]+')
$ echo $VERSION
4.12 4.12 4.12
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point, only searched relevant code in step ipi-conf-vsphere, and this part is indeed executed failed in upgrade job
2023-08-09 21:52:03+00:00 - sourcing context from vsphere_context.sh...
/bin/bash: line 39: [: too many arguments
2023-08-09 21:52:03+00:00 - unable to determine y stream, assuming this is master
2023-08-09 21:52:03+00:00 - 4.x installation is later than 4.9, will install with resource pool
2023-08-09 21:52:03+00:00 - pull-through cache force disabled
It is not safe to use this way to get version, might change to another way, e.g from release image.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think so.
if [ ! -z ${VERSION} ]; then | ||
Z_VERSION=$(echo ${VERSION} | cut -d'.' -f2) | ||
if [ ${Z_VERSION} -gt 13 ]; then | ||
cat >> "${ENABLE_TEMPLATE}" << EOF |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest to directly use "${SHARED_DIR}"/enable_template_content.txt
to replace ${ENABLE_TEMPLATE}
783b937
to
1b82fb3
Compare
|
||
MACHINE_POOL_OVERRIDES="" | ||
RESOURCE_POOL_DEF="" | ||
set +o errexit | ||
VERSION=$(echo "${JOB_NAME}" | grep -o -E '4\.[0-9]+') | ||
release="${installer_bin}" coreos print-stream-json |jq -r '.architectures.x86_64.artifacts.vmware.release' | ||
VERSION="${release:1:2}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
set -o errexit | ||
|
||
Z_VERSION=1000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest to keep the original var.
1b82fb3
to
15428d1
Compare
db3b143
to
92d66e2
Compare
@WenXinWei,
If the problem persists, please contact Test Platform. |
@WenXinWei: job(s): periodic-ci-openshift-openshift-tests-private-release-4.14-amd64-nightly-vsphere-ipi-template-enable-f14 either don't exist or were not found to be affected, and cannot be rehearsed |
1 similar comment
@WenXinWei: job(s): periodic-ci-openshift-openshift-tests-private-release-4.14-amd64-nightly-vsphere-ipi-template-enable-f14 either don't exist or were not found to be affected, and cannot be rehearsed |
92d66e2
to
c5f6c03
Compare
@WenXinWei,
If the problem persists, please contact Test Platform. |
/pj-rehearse |
c5f6c03
to
13d2448
Compare
/pj-rehearse |
@WenXinWei,
If the problem persists, please contact Test Platform. |
13d2448
to
59d4ba3
Compare
@WenXinWei,
If the problem persists, please contact Test Platform. |
/pj-rehearse |
[REHEARSALNOTIFIER]
A total of 580 jobs have been affected by this change. The above listing is non-exhaustive and limited to 35 jobs. A full list of affected jobs can be found here Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
@sgaoshang @WenXinWei do you know why "govc: Post "https://vcenter.devqe.ibmc.devcluster.openshift.com/sdk": dial tcp: lookup vcenter.devqe.ibmc.devcluster.openshift.com on 172.30.0.10:53: server misbehaving" always happened. |
/pj-rehearse periodic-ci-openshift-openshift-tests-private-release-4.14-amd64-nightly-vsphere-ipi-template-f14 |
not sure, i tried the govc command on my local. it works well. |
all the failed jobs are because e2e testing, so the PR is ready to merge. |
/pj-rehearse ack |
/lgtm |
cc @liangxia to review. |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jianlinliu, liangxia, WenXinWei 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:
Approvers can indicate their approval by writing |
@WenXinWei: The following tests failed, say
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. |
552e3e4
into
openshift:master
add step to config template in install-config.yaml and add workflow for enable template filed in installation.