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

Load the app/SP from the environment instead of automatically creating it #3498

Merged
merged 11 commits into from
Apr 7, 2024

Conversation

hawkowl
Copy link
Collaborator

@hawkowl hawkowl commented Apr 4, 2024

Which issue this PR addresses:

Part of ARO-5822

What this PR does / why we need it:

Loads the application and SP secret from the env, instead of automatically creating it. This allows us to load them from a fixed pool in the e2e process, instead of creating new ones each time.

Test plan for issue:

Manually tested creating a cluster locally with the steps updated in the docs

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

Updated dev RP docs

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

Purely CI/E2E/dev changes -- more will need to happen for prod e2e

@hawkowl
Copy link
Collaborator Author

hawkowl commented Apr 4, 2024

/azp run ci

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Collaborator

@mociarain mociarain left a comment

Choose a reason for hiding this comment

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

LGTM.

Did you manage to check that all the generated resources were indeed cleaned up?

.gitignore Show resolved Hide resolved
tsatam
tsatam previously approved these changes Apr 4, 2024
Copy link
Collaborator

@mociarain mociarain left a comment

Choose a reason for hiding this comment

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

Looking at it again I'm now very sure that removing the portal v1 line from the .gitignore will cause us issues

s-fairchild
s-fairchild previously approved these changes Apr 4, 2024
@hawkowl hawkowl dismissed stale reviews from s-fairchild and tsatam via d5cf596 April 5, 2024 02:51
@hawkowl
Copy link
Collaborator Author

hawkowl commented Apr 5, 2024

/azp run ci e2e

Copy link

No pipelines are associated with this pull request.

@hawkowl
Copy link
Collaborator Author

hawkowl commented Apr 5, 2024

/azp run ci, e2e

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@hawkowl
Copy link
Collaborator Author

hawkowl commented Apr 5, 2024

/azp run ci, e2e

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@hawkowl
Copy link
Collaborator Author

hawkowl commented Apr 5, 2024

/azp run ci, e2e

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@hawkowl
Copy link
Collaborator Author

hawkowl commented Apr 5, 2024

/azp run ci, e2e

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@hawkowl
Copy link
Collaborator Author

hawkowl commented Apr 5, 2024

/azp run ci, e2e

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@hawkowl
Copy link
Collaborator Author

hawkowl commented Apr 5, 2024

/azp run e2e, ci

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

.gitignore Show resolved Hide resolved
get_cluster_sp() {
echo "########## Downloading SP secrets ##########"

az keyvault secret download --vault-name=aro-e2e-principals \
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not blocking the PR, but we should add a TODO here for eventually grabbing a single SP among the entire pool

@hawkowl hawkowl merged commit 867b0d5 into master Apr 7, 2024
20 checks passed
@hawkowl hawkowl deleted the hawkowl/ARO-5822-load-sp branch April 7, 2024 22:06
jhoreman pushed a commit that referenced this pull request Apr 8, 2024
…g it (#3498)

* use multierror here, so it's more obvious if we're missing multiple keys

* Ignore the written out clusterapp.env

* move create/delete into separate commands, which write out a clusterapp.env file

* delete the app in the e2e.sh file

* update the docs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants