Skip to content

sample docker build file #640

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.

Sign up for GitHub

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

sample docker build file #640

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions docs/docker/ocpdiag.docker
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
FROM centos:latest
Copy link
Collaborator

Choose a reason for hiding this comment

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

does it have to be centos? can this run on a smaller distro like alpine?

Copy link
Author

Choose a reason for hiding this comment

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

I chose centos out of personal convenience, it doesn't have to be

Copy link
Collaborator

Choose a reason for hiding this comment

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

i should mention that this kinda depends on the usage; for demos, id say use alpine to make it smaller for distribution. for dev envs, centos can be fine because you'd normally build the container locally


###############################################################
#
# Sample docker build file to experiment with ocpdiag
#
# paste this in ocpdiag.docker
#
# docker build --rm ocpdiag.docker centos:ocpdiag
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this meant as a dev tool or demo?
if former, it would be better to use the more usual docker file structure <proj name>/Dockerfile, and that can be expanded on later. and move it somewhere else other than the ./docs folder (/tools or something maybe?)

if latter, id move it do ./docs/demos/ocpdiag.docker and put something else as entrypoint to showcase the demo, rather than a shell

Copy link
Author

Choose a reason for hiding this comment

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

I put this as a draft PR rather than shared as a code block in our other communication channels for ease of access by others. My intent was that when I went on to try to build a bare-bones "HelloWorld/SmokeTest" test for the quick start guide, I encountered an error with the bazel build step, I figured the easiest would be to share a reproducible environment for others to review the error.

I'm thinking more of a dev tool but something to discuss, not in its current form.

I used the docs folder as I figured we could have a doc section supporting examples on how to adopt and docker could be one way to onboard folks.

The entrypoint used is because I'm looking at this more as a runtime environment than a service or demo

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok, this wasnt clear to me. there was no message in the PR saying that this is just a request for comments thing and will not actually be merged

Copy link
Author

Choose a reason for hiding this comment

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

This was the first time I created a draft PR, https://github.blog/2019-02-14-introducing-draft-pull-requests/
it is a bit cryptic, I can see it tagged draft and at the bottom it has a button with "ready for review" but I agree it's not quite intuitive

#
# docker run -it --rm centos:ocpdiag
#
###############################################################

RUN sed -i 's/mirrorlist/#mirrorlist/g' /etc/yum.repos.d/CentOS-*
Copy link
Collaborator

Choose a reason for hiding this comment

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

comments on these? why are they removed?

Copy link
Author

Choose a reason for hiding this comment

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

this tweak is needed to circumvent docker bailing out at build time

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok but why?
what i was saying was that it's not obvious why you do this, so it would be useful to add a comment in the docker file explaining

Copy link
Author

Choose a reason for hiding this comment

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

I hear you and will add a comment if we decide to move this dockerfile forward. The reason is that centos 8 EOL'd and the mirrors were moved to vault.centos.org.

RUN sed -i 's|#baseurl=http://mirror.centos.org|baseurl=http://vault.centos.org|g' /etc/yum.repos.d/CentOS-*
RUN dnf install -y dnf-plugins-core
RUN dnf copr enable -y vbatts/bazel
RUN dnf install -y gcc
RUN dnf install -y gcc-c++
RUN dnf install -y unzip
RUN dnf install -y bazel4
RUN dnf install -y python3
RUN dnf install -y go
RUN python3 -m pip install dataclasses

COPY ocp-diag-core-main.zip /
RUN cd / && unzip ocp-diag-core-main.zip

WORKDIR /ocp-diag-core-main
RUN python3 ./apis/python/examples/demo.py 2>&1 > /ocpdiag_py_demo.log
Copy link
Collaborator

Choose a reason for hiding this comment

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

this seems like a rather odd usage of run commands; these are meant to build the image, but i get the feeling youre using them here as a demo (which also answers my question above). so by doing this youre baking in the results of these commands into the image, which has various implications.

what i think you should do is to add all of this to a shell script, pull that into the image and then use it as entrypoint (which may end up in the interactive shell if that's a goal)
this way, the image stays clean of artifacts that variate (like these log outputs) and docker can reuse it without rebuilding every time

Copy link
Author

Choose a reason for hiding this comment

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

Your prompt review of this beat me to setting up context, hopefully my explanation on the intent in an earlier comment explains how I approached this and why I put it out here as a draft.

So, yes, those executions are very much not intended as part of the build file and more of an example so others can reproduce the build error.

If deemed worthwhile I could see the validator become an image and the run of it could be parametrized to take test output as input

Copy link
Collaborator

Choose a reason for hiding this comment

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

let's revisit this after @dhawkes tells us about the dev docker file

RUN echo "exit 0" >> ./ci_scripts/run_ocpdiag_tests.sh
RUN ./ci_scripts/run_ocpdiag_tests.sh 2>&1 | tee -a /ocpdiag_build.log

WORKDIR /ocp-diag-core-main/validators/spec_validator
RUN go run . -schema=/ocp-diag-core-main/json_spec/output/validator.json ./samples/ 2>&1 | tee /ocpdiag_validator.out

ENTRYPOINT /bin/bash