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

inject sha256 labels for all target packages #90

Merged
merged 4 commits into from
Sep 25, 2020

Conversation

mathias-luedtke
Copy link
Contributor

@mathias-luedtke mathias-luedtke commented Sep 3, 2020

This PR addresses the SHA256 injection as proposed in osrf/docker_images#376

  • package handling functions have been refactored (breaks API) to parse the data only once
  • getPackageVersions returns list of dict(name, version, sha256) instead string
  • Dockerfile templates have been updated to use a common snippet
  • whitespace still differs a little bit
  • all packages get installed with apt-get install -y --no-install-recommends

I can update the common snippet to take a list of aptarguments, if needed.

@mathias-luedtke mathias-luedtke force-pushed the feature/inject-sha256-labels branch 2 times, most recently from 44562bb to 5c01008 Compare September 3, 2020 21:59
Copy link
Contributor

@mikaelarguedas mikaelarguedas left a comment

Choose a reason for hiding this comment

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

This looks great!

We should likely move some of these templates in a graveyard folder as some of them apply to images that will not be regenerated. (@ruffsl WDYT?)

Of the top of my head the changes to the following don't seem necessary (although don't hurt either):

  • docker_templates/templates/docker_images/create_drcsim_image.Dockerfile.em
  • docker_templates/templates/docker_images/create_gzserverX_image.Dockerfile.em

With or without the changes to the aforementioned files reverted, this PR looks good to me 👍

On the docker_images side, we'll likely need to update only a subset to not modify the Dockerfiles of the EOL images that are not rebuilt anymore. I'll comment directly on osrf/docker_images#454 for those comments

@ruffsl
Copy link
Member

ruffsl commented Sep 22, 2020

We should likely move some of these templates in a graveyard folder as some of them apply to images that will not be regenerated. (@ruffsl WDYT?)

Moving the template files would require the update of the paths in the respective images.yaml.em files. Perhaps best revisited in a separate PR.

@ruffsl ruffsl marked this pull request as ready for review September 22, 2020 23:38
@ruffsl ruffsl merged commit 90fdcbd into osrf:master Sep 25, 2020
@mikaelarguedas
Copy link
Contributor

Thanks again @ipa-mdl for working on this !

@ruffsl
Copy link
Member

ruffsl commented Sep 25, 2020

Thanks @ipa-mdl ! Next we'll need to work on automating the upstream PRs with our @osrf-docker-builder bot.

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.

None yet

3 participants