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.

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

ARO-4639 update the operator master deployment to support workload identity #3776

Merged
merged 3 commits into from
Sep 18, 2024

Conversation

yithian
Copy link
Collaborator

@yithian yithian commented Aug 16, 2024

Which issue this PR addresses:

Fixes ARO-4639

What this PR does / why we need it:

This causes the spec for the operator master deployment to mount the service account token as a volume, and maps the path to the environment variable expected by Azure to support workload identities

Test plan for issue:

Unit tests confirm that the generated yaml is valid and will be applied to a test cluster to confirm that it's a valid OpenShift resource as well

Is there any documentation that needs to be updated for this PR?

No, part of larger project of adding support for MIWI

How do you know this will function as expected in production?

As with all MIWI support work, it will be extremely challenging to test until we have a way to provision a MIWI cluster. Applying the updated generated resource to a test cluster will probably be the best we can do until then

@kimorris27
Copy link
Contributor

/azp run ci, e2e

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

Copy link
Contributor

@kimorris27 kimorris27 left a comment

Choose a reason for hiding this comment

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

LGTM as long as CI and E2E pass!

Copy link
Collaborator

@SudoBrendan SudoBrendan left a comment

Choose a reason for hiding this comment

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

I think there's a YAML syntax error on whitespace - dismiss if this is incorrect :)

Copy link

Please rebase pull request.

SudoBrendan
SudoBrendan previously approved these changes Aug 20, 2024
@yithian yithian added hold Hold chainsaw Pull requests or issues owned by Team Chainsaw labels Aug 20, 2024
@yithian
Copy link
Collaborator Author

yithian commented Aug 20, 2024

Putting a hold on this until #3761 is merged, as this was rebased onto that to pull in the token file location/mountpoint

@yithian
Copy link
Collaborator Author

yithian commented Aug 27, 2024

#3761 is merged, removing my hold

@yithian yithian removed the hold Hold label Aug 27, 2024
@github-actions github-actions bot added the needs-rebase branch needs a rebase label Aug 27, 2024
Copy link

Please rebase pull request.

This causes the spec for the operator master deployment to mount the
service account token as a volume, and maps the path to the environment
variable expected by Azure to support workload identities
pkg/operator/deploy/deploy_test.go Outdated Show resolved Hide resolved
pkg/operator/deploy/deploy.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@cadenmarchese cadenmarchese left a comment

Choose a reason for hiding this comment

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

makes sense to me!

@kimorris27
Copy link
Contributor

/azp run ci, e2e

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@cadenmarchese
Copy link
Collaborator

/azp run e2e

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Contributor

@kimorris27 kimorris27 left a comment

Choose a reason for hiding this comment

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

The only E2E failure is a known flake.

@cadenmarchese cadenmarchese merged commit bd47ae7 into master Sep 18, 2024
23 of 24 checks passed
@yithian yithian deleted the yithian/ARO-4639 branch September 19, 2024 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chainsaw Pull requests or issues owned by Team Chainsaw
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants