CORS-3931: [azure] add more permission when installing cluster in existing vnet #61374
Conversation
openshift-ci
bot
added
the
approved
|
/pj-rehearse pull-ci-openshift-installer-main-e2e-azure-ovn-shared-vpc |
|
@jinyunma: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
Installation on shared vpc succeeded in job /pj-rehearse ack cc @jianlinliu @gpei to review |
|
@jinyunma: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
openshift-ci-robot
added
the
rehearsals-ack
|
|
||
| # optional permissions when installing cluster in existing vnet | ||
| required_permissions=""" | ||
| \"Microsoft.Network/virtualNetworks/checkIpAddressAvailability/read\", |
There was a problem hiding this comment.
The permission is needed by recent PR change, but not for all the previous OCP versions, right?
There was a problem hiding this comment.
should be related with bug https://issues.redhat.com//browse/OCPBUGS-37442, which has been backported to 4.17. So this permission is required on 4.17+ CAPI install.
Will file doc bug to request to add the permission on 4.17+.
There was a problem hiding this comment.
Shall we place the needed permission into the if block on line 281?
There was a problem hiding this comment.
Error happens when creating manifests, this should impact both IPI and UPI.
But I can update to only apply permission on 4.17+.
There was a problem hiding this comment.
@jinyunma one more question, does it only affect the installation for existing vnet? If so, do we need a separate env var to control it, so we can skip this permission for IPI install without existing vnet?
There was a problem hiding this comment.
Good suggestion.
As discussed offline, will read install-config and assign proper permissions based on various config.
jinyunma
force-pushed
the
az-byo-vpc-permission
branch
from
11e52b0 to
69a16bb
Compare
|
/lgtm |
openshift-ci-robot
removed
the
rehearsals-ack
|
/pj-rehearse ack |
|
@jinyunma: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
openshift-ci-robot
added
the
rehearsals-ack
|
/hold |
openshift-ci
bot
added
the
do-not-merge/hold
jinyunma
force-pushed
the
az-byo-vpc-permission
branch
from
69a16bb to
d85e505
Compare
openshift-ci
bot
removed
lgtm
openshift-ci-robot
removed
the
rehearsals-ack
jinyunma
force-pushed
the
az-byo-vpc-permission
branch
from
d85e505 to
70960ab
Compare
|
/pj-rehearse pull-ci-openshift-installer-main-azure-ovn-marketplace-images periodic-ci-openshift-openshift-tests-private-release-4.18-multi-nightly-azure-ipi-des-mini-perm-fips-amd-f28-destructive periodic-ci-openshift-openshift-tests-private-release-4.18-multi-nightly-azure-upi-mini-perm-amd-f28-destructive periodic-ci-openshift-openshift-tests-private-release-4.18-amd64-nightly-azure-ipi-marketplace-mini-perm-f7 |
|
@jinyunma: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
...taller-rehearse-azure-ipi-cco-manual-workload-identity-managed-identity-provision-chain.yaml
Outdated
Show resolved
Hide resolved
jinyunma
force-pushed
the
az-byo-vpc-permission
branch
2 times, most recently
from
b32ff88 to
34d925a
Compare
|
/pj-rehearse pull-ci-openshift-installer-main-e2e-azure-ovn-shared-vpc periodic-ci-openshift-openshift-tests-private-release-4.18-multi-nightly-azure-ipi-des-mini-perm-fips-amd-f28-destructive periodic-ci-openshift-openshift-tests-private-release-4.18-multi-nightly-azure-upi-mini-perm-amd-f28-destructive periodic-ci-openshift-openshift-tests-private-release-4.18-multi-nightly-azure-ipi-oidc-mini-perm-arm-f7 |
|
/approve |
1 similar comment
|
/approve |
|
cc @liangxia to approve |
|
/lgtm |
|
/pj-rehearse ack |
|
@jinyunma: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
openshift-ci-robot
added
the
rehearsals-ack
| - ref: azure-provision-service-principal-minimal-permission | ||
| - ref: ipi-conf | ||
| - ref: ipi-conf-azure-default | ||
| - chain: azure-provision-service-principal-minimal-permission |
There was a problem hiding this comment.
Do we really want to change openshift-priv ones ?
There was a problem hiding this comment.
Are those openshift-priv jobs automatically synced up by some way? I searched them when updating this chain.
I'm not sure when this job will be triggered. Once triggered, the change is needed here.
There was a problem hiding this comment.
Yeah, we updated the openshift-priv ones on purpose because of mini-perm change.
There was a problem hiding this comment.
AFAIK, those openshift-priv are used by ART team. Generally we don't touch them as there are auto sync mechanism.
If you are sure this is exactly needed, I can approve it. Just let me know.
There was a problem hiding this comment.
got it, the change is required regardless in which way to update them.
Thanks for approving this.
There was a problem hiding this comment.
@liangxia I removed the change in openshift-priv job config, PTAL.
There was a problem hiding this comment.
seems that the change must be done manually in this PR for openshift-priv jobs, otherwise job pull-ci-openshift-release-master-ci-operator-config failed, because some envs are removed on purpose in this PR.
So I changed back for openshift-priv jobs.
jinyunma
force-pushed
the
az-byo-vpc-permission
branch
from
ae630e2 to
4ac0a99
Compare
jinyunma
force-pushed
the
az-byo-vpc-permission
branch
from
4ac0a99 to
c98d27d
Compare
openshift-ci-robot
removed
the
rehearsals-ack
|
/approve |
openshift-ci
bot
added
the
approved
jinyunma
force-pushed
the
az-byo-vpc-permission
branch
from
c98d27d to
e0ba71a
Compare
|
@jinyunma: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
|
[REHEARSALNOTIFIER]
A total of 2467 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: |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gpei, jianlinliu, jinyunma, liangxia, patrickdillon The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/pj-rehearse ack |
|
@jinyunma: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
openshift-ci-robot
added
the
rehearsals-ack
6 participants
Add additional permission "Microsoft.Network/virtualNetworks/checkIpAddressAvailability/read" to fix the issue
And introduce new chain "azure-provision-service-principal-minimal-permission" to include both steps.
azure-provision-disk-encryption-set,azure-provision-vnet,ipi-conf-azure-resourcegroupas separated steps, only run role assignment steps in dedicated jobs instead of using ENV to control it.