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

fix incorrect download bundle #200

Merged
merged 5 commits into from
Oct 30, 2023

Conversation

snagoor
Copy link
Contributor

@snagoor snagoor commented Sep 27, 2023

What does this PR do?

Incorrect setup bundle is downloaded with 2.4 and RHEL9 version ansible-automation-platform-containerized-setup

How should this be tested?

Detailed bug report is provided here #199

Is there a relevant Issue open for this?

Provide a link to any open issues that describe the problem you are solving.
resolves #199

Other Relevant info, PRs, etc

Please provide link to other PRs that may be related (blocking, resolves, etc. etc.)

Copy link
Member

@kubealex kubealex left a comment

Choose a reason for hiding this comment

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

To make it future-proof, it could be worth making the check more robust ensuring that the whole package name reflects the required "ansible-automation-platform-setup-bundle" or "ansible-automation-platform-setup" pattern.
By doing this you can safely assume that you are always picking the 'supported' installer bundle.

@snagoor
Copy link
Contributor Author

snagoor commented Sep 27, 2023

Thank you for the review. Updated the logic to check ansible-automation-platform-setup in file name.

@snagoor snagoor requested a review from kubealex September 27, 2023 14:57
@jinalklaulitz
Copy link

jinalklaulitz commented Sep 28, 2023

Could also please remove or increase the elements on the list slice for __aap_setup_down_images in line 34; at the loop statement for name: Downloading the latest installer of type {{ aap_setup_down_type }}? I'm working with the setup bundle and it was being filtered out since it has more elements now due to the list now including the setup and setup bundle for the containerized setup.

@snagoor
Copy link
Contributor Author

snagoor commented Oct 20, 2023

see also PR #207 which should fix both #199 and #206

Copy link
Contributor

@djdanielsson djdanielsson left a comment

Choose a reason for hiding this comment

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

I am not positive the addition is required with the other change but I guess it doesn't hurt either.

@djdanielsson djdanielsson enabled auto-merge (squash) October 30, 2023 18:32
@djdanielsson djdanielsson merged commit 8703ce4 into redhat-cop:devel Oct 30, 2023
8 checks passed
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.

Incorrect installation setup bundle extracted and configured
4 participants