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

Build docker images using the build arguments instead of sed'ing Dockerfile #11646

Closed
amaltaro opened this issue Jul 5, 2023 · 4 comments · Fixed by #11638, dmwm/CMSKubernetes#1397, #11651, #11652 or #11653

Comments

@amaltaro
Copy link
Contributor

amaltaro commented Jul 5, 2023

Impact of the new feature
WMCore CD pipeline

Is your feature request related to a problem? Please describe.
Just because I already started this work:
#11638

I'd rather not trash bin it.

Describe the solution you'd like
Instead of replacing the TAG - via sed command - directly in the WMCore Dockerfiles available under:
https://github.com/dmwm/CMSKubernetes/tree/master/docker/pypi

use the docker --build-arg to pass the correct tag in the command line.

Describe alternatives you've considered
Keep sed around

Additional context
None

@todor-ivanov
Copy link
Contributor

Hi @amaltaro

Just because I already started this work: #11638 I'd rather not trash bin it.

I completely agree with you. Lets push this to the end.

@amaltaro
Copy link
Contributor Author

amaltaro commented Jul 7, 2023

Sigh, reopening this! The implementation works as expected, but it turns out that when the GH workflow is going to publish the new image to the CERN registry, it loads the Dockerfile once again, which results in a TAG=None.

There is no specific docker build/run in the GH workflow:
https://github.com/dmwm/WMCore/blob/master/.github/workflows/docker_images_template.yaml#L49

so I have to research what needs to be done in this case.

@amaltaro
Copy link
Contributor Author

amaltaro commented Jul 8, 2023

I will have to revisit this docker build GH workflow because:

  1. with the current changes, it still fails for reqmgr2ms-unmerged (see Rename build_args in GH action workflow #11652 for further details)
  2. the docker action v1 is being deprecated
  3. it is not efficient, as it builds the image twice
  4. apparently the Login step is not required

For that, I better setup a private repository to test it, otherwise it will be a bunch of useless commits until finally converging. Apologies for that mess!

@amaltaro amaltaro reopened this Jul 8, 2023
@todor-ivanov
Copy link
Contributor

todor-ivanov commented Jul 8, 2023

Hi @amaltaro
Just to link here some more details I wrote last week related to few of the bullets from above:

  1. the docker action v1 is being deprecated

#11642 - pay attention to the NOTE in the PR description, when upgrading to docker action v2 about the changing of the path to context parameter. Please also do not overlook the fact that the repsitory parameter is deprecated and will affect how we create those image tags before uploading.

  1. it is not efficient, as it builds the image twice

#11641 - I tried to put a short description of my findings on how those two builds happen one after another. I think the reason why we have ended up with that is because the docker/build module usually considers the Docker files/context to be provided within the current githup repository, while we download/fetch them from CMSkubernetes. I am pretty sure we may leave the first step to only cover the Docker context downloading and within the docker/build-upload module/action we can do the full build and upload (provided we have properly tagged them).

  1. apparently the Login step is not required
    Well in most of the examples I've seen in the net, the login step is actually present, while the user and password parameters to the docker/build-upload action are skipped.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment