Skip to content
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

Jump to bottom

[OCPCLOUD-1541] Bug 2089334: Use service account credentials for all providers #193

Merged
merged 1 commit into from
May 30, 2022
Merged

[OCPCLOUD-1541] Bug 2089334: Use service account credentials for all providers #193

merged 1 commit into from
May 30, 2022

Conversation

Fedosin
Copy link
Contributor

@Fedosin Fedosin commented May 19, 2022 •
edited

OpenShift enforces the --use-service-account-credentials flag on CCMs.

This commit adds a test that makes sure this is configured correctly on each cloud provider except vSphere platform.

On vSphere they changed the behavior from what they have in the in-tree cloud provider. Code in this CCM executed by the service-controller runs as vsphere-cloud-controller-manager in the kube-system namespace, which is a dynamically created but hardcoded service account. But in the in-tree cloud provider the client is called vsphere-legacy-cloud-provider

JoelSpeed
JoelSpeed reviewed May 23, 2022
View reviewed changes
Copy link
Contributor

@JoelSpeed JoelSpeed left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@openshift-ci openshift-ci bot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels May 23, 2022
@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

3 similar comments
@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@JoelSpeed JoelSpeed changed the title [OCPCLOUD-1541] Use service account credentials for all providers [OCPCLOUD-1541] Bug 2089334: Use service account credentials for all providers May 23, 2022
@openshift-ci openshift-ci bot added bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels May 23, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 23, 2022

@Fedosin: This pull request references Bugzilla bug 2089334, 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
  • bug is open, matching expected state (open)
  • bug target release (4.11.0) matches configured target release for branch (4.11.0)
  • bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @sunzhaohua2

In response to this:

[OCPCLOUD-1541] Bug 2089334: Use service account credentials for all providers

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 openshift-ci bot requested a review from sunzhaohua2 May 23, 2022 13:08
@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

14 similar comments
@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@JoelSpeed
Copy link
Contributor

/test e2e-aws-ccm
/test e2e-vsphere-ccm-install
/test e2e-vsphere-ccm

@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

2 similar comments
@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@JoelSpeed
Copy link
Contributor

/hold
/lgtm cancel
/approve cancel

The CI is showing that the vsphere CCM is not working correctly, the clusters have not completed bootstrapping as the nodes are left uninitialized, @Fedosin can you investigate this by spinning up your own vSphere cluster and inspecting why the pod isn't working correctly?

@openshift-ci openshift-ci bot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. and removed lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels May 24, 2022
@Fedosin
Copy link
Contributor Author

Fedosin commented May 24, 2022

/test e2e-vsphere-ccm-install

@Fedosin
Copy link
Contributor Author

Fedosin commented May 24, 2022

/retest

@Fedosin
[OCPCLOUD-1541] Use service account credentials for all providers
ad0e969 
OpenShift enforces the `--use-service-account-credentials` flag on
CCMs.

This commit adds a test that makes sure this is configured correctly
on each cloud provider.
@Fedosin Fedosin force-pushed the use_service_account_credentials branch from 2c9a2e7 to ad0e969 Compare May 25, 2022 12:38
@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 25, 2022 •
edited

@Fedosin: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-azure-ccm ad0e969 link false /test e2e-azure-ccm
ci/prow/e2e-vsphere-ccm-install ad0e969 link false /test e2e-vsphere-ccm-install
ci/prow/e2e-vsphere-ccm ad0e969 link false /test e2e-vsphere-ccm
ci/prow/e2e-gcp-ccm-install ad0e969 link false /test e2e-gcp-ccm-install
ci/prow/e2e-azure-upgrade ad0e969 link false /test e2e-azure-upgrade

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.

@lobziik
Copy link
Contributor

lobziik commented May 26, 2022

/approve

@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 26, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lobziik

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 26, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 27, 2022

@Fedosin: This pull request references Bugzilla bug 2089334, which is valid.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.11.0) matches configured target release for branch (4.11.0)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @sunzhaohua2

In response to this:

[OCPCLOUD-1541] Bug 2089334: Use service account credentials for all providers

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.

@JoelSpeed
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label May 30, 2022
@Fedosin
Copy link
Contributor Author

Fedosin commented May 30, 2022

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 30, 2022
@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 2 against base HEAD 8dd06f0 and 8 for PR HEAD ad0e969 in total

@openshift-merge-robot openshift-merge-robot merged commit c2f2cbf into openshift:master May 30, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 30, 2022

@Fedosin: All pull requests linked via external trackers have merged:

Bugzilla bug 2089334 has been moved to the MODIFIED state.

In response to this:

[OCPCLOUD-1541] Bug 2089334: Use service account credentials for all providers

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.

@dulek dulek mentioned this pull request Apr 11, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JoelSpeed JoelSpeed JoelSpeed left review comments

@alexander-demicev alexander-demicev Awaiting requested review from alexander-demicev

@sunzhaohua2 sunzhaohua2 Awaiting requested review from sunzhaohua2

Assignees

@JoelSpeed JoelSpeed

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@Fedosin @openshift-bot @JoelSpeed @lobziik @openshift-ci-robot @openshift-merge-robot