Bug 1868339: Move CredentialsRequest creation to CSO #80
Conversation
openshift-ci-robot
added
bugzilla/severity-medium
|
@bertinatto: This pull request references Bugzilla bug 1868339, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker. 3 validation(s) were run on this bug
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/test-infra repository. |
openshift-ci-robot
added
the
approved
|
@bertinatto: This pull request references Bugzilla bug 1868339, which is valid. 3 validation(s) were run on this bug
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/test-infra repository. |
|
/hold Before this merges I need to patch the CSI driver operators and then test manually. |
openshift-ci-robot
added
the
do-not-merge/hold
|
Also, @dgoodwin FYI |
|
Thank you for this, looks good from my PoV. |
bertinatto
force-pushed
the
fix-credentials
branch
from
87b75d3
to
d49f87e
Compare
|
@bertinatto: This pull request references Bugzilla bug 1868339, which is valid. 3 validation(s) were run on this bug
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/test-infra repository. |
|
/retest |
|
/test e2e-aws
|
@bertinatto Can CCO handle cases where secret was already created? Looks like it already does that otherwise this PR would have failed test I think. |
Yes, from what I've seen it can handle that. |
| @@ -0,0 +1,31 @@ | |||
| apiVersion: cloudcredential.openshift.io/v1 | |||
| kind: CredentialsRequest | |||
There was a problem hiding this comment.
should this be called "ebs-credential-request"? In case we have to create separate credential request for EFS for example?
There was a problem hiding this comment.
Changed to aws-ebs-csi-driver-operator to align with the other CCs which are using the operator name.
| spec: | ||
| secretRef: | ||
| name: aws-cloud-credentials | ||
| namespace: openshift-cluster-csi-drivers |
There was a problem hiding this comment.
same applies to secret's name too.
There was a problem hiding this comment.
We'll need to have a coordinated way of changing the secret name here and in the CSI driver operator repo.
If I change it here, the CI job will likely fail because the CSI driver is looking for this secret name.
There was a problem hiding this comment.
Renamed to ebs-cloud-credentials (similar to Manila and oVirt).
It shouldn't fail CI because for the time being the CSI driver operator is creating the secret itself.
CredentialRequests objects have been moved to ./manifests dir, so they will be applied by CSO regardless of the cloud. Therefore, CSI operators should ot create the credentials themselves. This patch is a requirement to allow cloud-credentials-controller to run in Manual mode. More information at https://github.com/openshift/cloud-credential-operator/blob/master/docs/mode-manual-creds.md
bertinatto
force-pushed
the
fix-credentials
branch
from
d49f87e
to
242ae2c
Compare
|
/hold cancel CSI driver operators should be changed after this PR merges. |
openshift-ci-robot
removed
the
do-not-merge/hold
So that the openshift-cluster-csi-drivers namespace exists when the cloud-credential-operator deploys the secret.
bertinatto
force-pushed
the
fix-credentials
branch
from
242ae2c
to
2c521c2
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bertinatto, gnufied, tsmetana 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 |
|
/retest |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
@bertinatto: All pull requests linked via external trackers have merged: Bugzilla bug 1868339 has been moved to the MODIFIED state. 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/test-infra repository. |
CSI operators should let CSO apply the CredentialsRequest objects, regardless of the cloud platform.
This patch is a requirement to allow users to manually see the secrets they need to create when running cloud-credentials-controller in Manual mode. More information at: https://github.com/openshift/cloud-credential-operator/blob/master/docs/mode-manual-creds.md.
Once this is merged, CredentialsRequests will be created by CVO, so we'll need follow up PRs to CSI operator repositories to not create those objects themselves.
Please see individual commits for a description of each change.
CC @openshift/storage