Skip to content

azure: upi: sync install workflow with installer changes #34678

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

Merged
openshift-merge-robot merged 1 commit into from
Dec 16, 2022
Merged

azure: upi: sync install workflow with installer changes #34678

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
25 changes: 20 additions & 5 deletions ci-operator/step-registry/upi/install/azure/upi-install-azure-commands.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,11 @@ az storage account create -g $RESOURCE_GROUP --location $AZURE_REGION --name $AC
ACCOUNT_KEY=$(az storage account keys list -g $RESOURCE_GROUP --account-name $ACCOUNT_NAME --query "[0].value" -o tsv)

if openshift-install coreos print-stream-json 2>/tmp/err.txt >/tmp/coreos.json; then
VHD_URL="$(jq -r '.architectures.x86_64."rhel-coreos-extensions"."azure-disk".url' /tmp/coreos.json)"
VHD_URL="$(jq -r --arg arch "$OCP_ARCH" '.architectures[$arch]."rhel-coreos-extensions"."azure-disk".url' /tmp/coreos.json)"
RELEASE_VERSION="$(jq -r --arg arch "$OCP_ARCH" '.architectures[$arch]."rhel-coreos-extensions"."azure-disk".release' /tmp/coreos.json)"
else
VHD_URL="$(jq -r .azure.url /var/lib/openshift-install/rhcos.json)"
RELEASE_VERSION="$(jq -r .azure.release /var/lib/openshift-install/rhcos.json)"
fi

echo "Copying VHD image from ${VHD_URL}"
Expand Down Expand Up @@ -218,10 +220,23 @@ az network private-dns link vnet create -g $RESOURCE_GROUP -z ${CLUSTER_NAME}.${

echo "Deploying 02_storage"
VHD_BLOB_URL=$(az storage blob url --account-name $ACCOUNT_NAME --account-key $ACCOUNT_KEY -c vhd -n "rhcos.vhd" -o tsv)
az deployment group create -g $RESOURCE_GROUP \
--template-file "02_storage.json" \
--parameters vhdBlobURL="${VHD_BLOB_URL}" \
--parameters baseName="$INFRA_ID"

# Check if it's the new template using Image Galleries instead of Managed Images
if grep -qs "Microsoft.Compute/galleries" 02_storage.json; then
Copy link
Contributor

@patrickdillon patrickdillon Dec 16, 2022

Choose a reason for hiding this comment

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

I am reluctant to bring this up without a better suggestion, but I worry that putting conditionals in a template like this can be risky in terms of legibility/debugability. Our old system of templates became so convoluted that it was hard to follow the execution path, so i am perhaps overly hesitant whenever I see any conditionals.

The change introduced here is simple, so I think it is better than any of the alternatives I can come up with, which would be overkill or more confusing than this.

Copy link
Contributor Author

@r4f4 r4f4 Dec 16, 2022

Choose a reason for hiding this comment

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

This was the best idea I could come up with to make this work with both Managed Images and Galleries. Relying on the installer or release payload versions didn't feel reliable enough. Once 4.12 migrates to Galleries and this workflow is run on 4.12+, then we can drop this conditional. But I agree with you this is not ideal.

Copy link
Contributor Author

@r4f4 r4f4 Dec 21, 2022

Choose a reason for hiding this comment

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

@patrickdillon @barbacbd pointed out an interesting idea. We could use the contentVersion from the ARM template [1] [2]. It still a conditional but at least it checks for a version instead of a string inside the template. Then it's up to the Installer team to make sure that version is updated accordingly when there are changes to the templates.

[1] https://github.com/openshift/installer/blob/master/upi/azure/02_storage.json#L3
[2] https://learn.microsoft.com/en-us/azure/azure-resource-manager/templates/syntax#template-format

AZ_ARCH=$(echo $OCP_ARCH | sed 's/x86_64/x64/;s/aarch64/Arm64/')
az deployment group create -g $RESOURCE_GROUP \
--template-file "02_storage.json" \
--parameters vhdBlobURL="${VHD_BLOB_URL}" \
--parameters baseName="$INFRA_ID" \
--parameters storageAccount="$ACCOUNT_NAME" \
--parameters imageRelease="${RELEASE_VERSION::15}" \
--parameters architecture="$AZ_ARCH"
else
az deployment group create -g $RESOURCE_GROUP \
--template-file "02_storage.json" \
--parameters vhdBlobURL="${VHD_BLOB_URL}" \
--parameters baseName="$INFRA_ID"
fi

echo "Deploying 03_infra"
az deployment group create -g $RESOURCE_GROUP \
Expand Down
5 changes: 5 additions & 0 deletions ci-operator/step-registry/upi/install/azure/upi-install-azure-ref.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,10 @@ ref:
requests:
cpu: 10m
memory: 100Mi
env:
- name: OCP_ARCH
default: "x86_64"
documentation: |-
The architecture of the control plane nodes (e.g., x86_64, aarch64)
documentation: >-
This step deploys a UPI cluster to the CI Azure project.