Skip to content

Action for building docker images automatically#1441

Merged
j143 merged 7 commits intoapache:mainfrom
j143:docker-action-pub
Jan 8, 2022
Merged

Action for building docker images automatically#1441
j143 merged 7 commits intoapache:mainfrom
j143:docker-action-pub

Conversation

@j143
Copy link
Member

@j143 j143 commented Nov 6, 2021

Tasks

  • 1. Run nightly build as cron job
  • 2. run test build for each commit
  • 3. version build for release version

How to Review?

Published artifact at https://hub.docker.com/r/return01/test-ghactions-systemds

Usage test:

janardhan@cloudshell:~ (ai-experiments-001)$ docker run -it --rm return01/test-ghactions-systemds
Hello from SystemDS
SystemDS Statistics:
Total execution time:           0.020 sec.

Secrets: DOCKERHUB_USER and DOCKERHUB_TOKEN
These two variables are already set by INFRA. No need to set from our end.

https://issues.apache.org/jira/browse/INFRA-22430

@Baunsgaard
Copy link
Contributor

I think we can split the images a bit and reduce builds.

  1. Build max once a day.
  2. Only build the test image if we change R dependencies or the pom.xml
  3. Only build the other images if there is changes in src.

Thanks for the help!

@j143 j143 requested a review from Baunsgaard November 24, 2021 16:19
@j143 j143 changed the title [WIP] Action for building docker images automatically Action for building docker images automatically Nov 24, 2021
- name: Checkout
uses: actions/checkout@v2

- name: Set up QEMU
Copy link
Contributor

Choose a reason for hiding this comment

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

QEMU is for ?

Copy link
Member Author

@j143 j143 Nov 26, 2021

Choose a reason for hiding this comment

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

It is an add-on, incase we want to build multi arch images. Optional though!

https://github.com/docker/buildx/#building-multi-platform-images

usage

$ docker buildx build --platform linux/amd64,linux/arm64 .

I do not have a about whether we need this. Perhaps, federated feature
might need this (I do not have good understanding yet).

Copy link
Contributor

Choose a reason for hiding this comment

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

hmm, then for now lets not include it if it is not needed

rm -r target/maven-archiver && \
rm -r target/systemds-** && \
rm -r docker && \
# rm -r docker && \
Copy link
Contributor

Choose a reason for hiding this comment

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

you cannot just comment out a line in this multi-line instruction, then everything after it does not execute.

Copy link
Member Author

@j143 j143 Nov 26, 2021

Choose a reason for hiding this comment

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

Suggested change
# rm -r docker && \
rm -r docker && \

Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure what you mean?
just for information the reason why i have this line is to remove the docker folder from the internal copied systemds repository, it does not make a difference for the building of the docker image except it makes the end result slightly smaller.

so therefore, don't out comment it :)

COPY docker/mountFolder/main.dml /input/main.dml
RUN mkdir /input && echo 'print("Hello from SystemDS")' > /input/main.dml

# COPY docker/mountFolder/main.dml /input/main.dml
Copy link
Contributor

Choose a reason for hiding this comment

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

i'm all for running it but don't remove the copy, so that the only change you make it to add the RUN line.

Copy link
Member Author

Choose a reason for hiding this comment

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

I am having error with running COPY

Copy link
Member Author

Choose a reason for hiding this comment

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

This one

image

Copy link
Contributor

Choose a reason for hiding this comment

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

this is happening because of your out commented line mentioned above.

Copy link
Contributor

Choose a reason for hiding this comment

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

this one:

# rm -r docker && \

@Baunsgaard
Copy link
Contributor

I think it is fine if we just add the on commit part of the docker for now,
If it is still working, we should start merging it in.

@Baunsgaard
Copy link
Contributor

@j143
Is the only thing missing not just a Infra ticket to add keys to push the images?

@j143
Copy link
Member Author

j143 commented Jan 8, 2022

Is the only thing missing not just a Infra ticket to add keys to push the images?

There are already secrets available.

the problem seems to be COPY docker/mountFolder/main.dml /input/main.dml statement in the Dockerfile. Let me give a couple of hours to it. :)

#9 [4/4] COPY docker/mountFolder/main.dml /input/main.dml
#9 ERROR: failed to walk /tmp/buildkit-mount645512887/docker/mountFolder: lstat /tmp/buildkit-mount645512887/docker/mountFolder: no such file or directory

@j143 j143 merged commit de8a342 into apache:main Jan 8, 2022
@j143 j143 deleted the docker-action-pub branch January 8, 2022 16:53
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.

2 participants