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

unixPb: run the updatepackage.sh script via dockerhost playbook #3152

Merged
merged 3 commits into from
Aug 11, 2023

Conversation

Haroon-Khel
Copy link
Contributor

@Haroon-Khel Haroon-Khel commented Aug 3, 2023

  • commit message has one of the standard prefixes
  • faq.md updated if appropriate
  • other documentation is changed or added (if applicable)
  • playbook changes run through VPC or QPC (if you have access)
  • VPC/QPC not applicable for this PR
  • for inventory.yml changes, bastillion/nagios/jenkins updated accordingly

ref #2962

Ive added a job in AWX to run the updatepackage.sh script regularly (will initiate it when this gets merged). This is better than it being in a cron job since AWX will pull updates to the script. It will run tasks in the dockerhost.yml playbook tagged with updateContainers which only corresponds to the task I have added in this pr. The never tag ensures that the task will only run when specified (by use of the updateContainers tag) and not as part of the entire playbook.

This is still a draft because though this script updates the containers, it does not install new packages if needed, so I need to think of a clean way to do that.

My last resort is to add strings in the updatepackages.sh script which contain any new package that needs to be installed on the containers (sort of how we do in the Common role but as bash strings) but im certain theres a better way to do this.

Any thoughts @sxa @steelhead31

@Haroon-Khel Haroon-Khel marked this pull request as draft August 3, 2023 12:31
@steelhead31
Copy link
Contributor

@Haroon-Khel I guess you could include a config file alongside the update packages.sh script itself, and have that script read in from the config file. I'm not entirely convinced its a better solution, but it would mean changes to which packages wouldn't involve changing the driving script, ultimately its not that different.

@github-actions github-actions bot added docker and removed docker labels Aug 10, 2023
@Haroon-Khel Haroon-Khel marked this pull request as ready for review August 10, 2023 14:31
Copy link
Contributor

@steelhead31 steelhead31 left a comment

Choose a reason for hiding this comment

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

LGTM!

@Haroon-Khel
Copy link
Contributor Author

Ive added some packages to be maintained/installed (depending on its presence). I think it would be overkill to add every package we install. I will continue to improve the script, but for now this works as a means to keep our containers updated (via the awx job) and to add new packages to containers when we need to

Copy link
Member

@sxa sxa left a comment

Choose a reason for hiding this comment

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

While it solves the immediate problem and I'm ok with letting this in for now I'm not sure this is the best way to go about it long term as we'll be adding to it every time we have a new one and end up with a partial duplication of package names here and in the docker files. Could we pull the list from the Dockerfiles?

I almost feel like we should be rebuilding the containers when a new package is added if possible.

(Also just as a point of note, the referenced issue #2962 was to come up with the process/policy proposal(s) first - implementation was intended to be a separate thing ;-) )

@Haroon-Khel Haroon-Khel merged commit 9bf2677 into adoptium:master Aug 11, 2023
6 of 9 checks passed
@sxa sxa added this to the 2023-08 (August) milestone Aug 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants