enable HYPERSHIFT_AZURE_ASSIGN_SERVICE_PRINCIPAL_ROLES and remove skip-service-principal-deletion param #60346
Conversation
heliubj18
commented
Jan 6, 2025
heliubj18
commented
- enable HYPERSHIFT_AZURE_ASSIGN_SERVICE_PRINCIPAL_ROLES
- remove skip-service-principal-deletion param to fix e2e failures
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: heliubj18 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
...vision/service-principal/hypershift/azure-provision-service-principal-hypershift-commands.sh
Show resolved
Hide resolved
heliubj18
force-pushed
the
enable-auto-assign-role
branch
from
8d00731 to
dd3289a
Compare
|
|
||
| client_id="$(eval "az ad sp create-for-rbac --name $name --role Contributor --scopes $scopes --create-cert --cert $name --keyvault $KV_NAME --output json --only-show-errors" | jq -r '.appId')" | ||
| client_id="$(eval "az ad sp create-for-rbac --name $name --create-cert --cert $name --keyvault $KV_NAME --output json --only-show-errors" | jq -r '.appId')" | ||
| echo "$client_id" >> "${SHARED_DIR}/azure_sp_id" | ||
|
|
||
| component_to_client_id+=(["$component"]="$client_id") | ||
| component_to_cert_name+=(["$component"]="$name") | ||
| done | ||
|
|
||
| # TODO: Remove this once the we used the automated role assignment by "--assign-service-principal-role" |
There was a problem hiding this comment.
Should this whole block just be removed now?
There was a problem hiding this comment.
Here it prepares one managed identity file hypershift_azure_mi_file.json used in the azure provision chain when HYPERSHIFT_AZURE_CP_MI is true and it is also needed by HYPERSHIFT_AZURE_ASSIGN_SERVICE_PRINCIPAL_ROLES
https://github.com/openshift/hypershift/blob/92bbcedfa4951be1c4edbba6465342a37fb6fda7/cmd/infra/azure/create.go#L223
And in the hypershift validation, this HYPERSHIFT_AZURE_CP_MI is needed when TechPreviewEnabled is enabled. So I have to keep that logic here.
https://github.com/openshift/hypershift/blob/001b0daa3ceecb87ab0a3388b4cff40338b7d633/cmd/cluster/azure/create.go#L141
There was a problem hiding this comment.
@bryan-cox One question: If we don't prepare the service principal in advance, is there another step or a specific area within Hypershift that handles the creation of service principals? Thank you!
There was a problem hiding this comment.
No, we removed the creation of SPs on purpose as it contributes to Azure quota issues. The direction from Red Hat and Microsoft were to reuse SPs as much as we can.
There was a problem hiding this comment.
OK, I will prepare some permanent SPs manually and store them into the vault to use them in the step. thanks
heliubj18
force-pushed
the
enable-auto-assign-role
branch
from
dd3289a to
33190d4
Compare
|
/pj-rehearse periodic-ci-openshift-openshift-tests-private-release-4.19-amd64-nightly-azure-aks-hypershift-ephemeral-creds-guest-f7 |
|
@heliubj18: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
heliubj18
force-pushed
the
enable-auto-assign-role
branch
from
33190d4 to
1646369
Compare
|
[REHEARSALNOTIFIER]
A total of 60 jobs have been affected by this change. The above listing is non-exhaustive and limited to 25 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: |
|
@heliubj18: The following test 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-sigs/prow repository. I understand the commands that are listed here. |
|
it can not work with HYPERSHIFT_AZURE_ASSIGN_SERVICE_PRINCIPAL_ROLES due to lack of az client in the HO image. |
|
@heliubj18: Closed this PR. In response to this:
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-sigs/prow repository. |
