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

add --latest-only caching option #232

Merged

Conversation

nickboldt
Copy link
Contributor

Change-Id:...

add --latest-only caching option

Change-Id: Ie0690a1ab8f9e89989b658a05b890075635c92e6
Signed-off-by: nickboldt nboldt@redhat.com

…/che does correct file assoc

Change-Id: I4e12db0a173fc7aeb59947139ba39a01c6b227d0
Signed-off-by: nickboldt <nboldt@redhat.com>
…Dockerfile.caching

Change-Id: Ib097d689276c68b45cac707c23efd2332ae7d6c4
Signed-off-by: nickboldt <nboldt@redhat.com>
Change-Id: I3c7d4f11774261b7110f44e58f6460440855b486
Signed-off-by: nickboldt <nboldt@redhat.com>
Change-Id: Ie0690a1ab8f9e89989b658a05b890075635c92e6
Signed-off-by: nickboldt <nboldt@redhat.com>
…nneeded USER commands

Change-Id: Iff3aa7c7cc9b912b8f435e0932e8c0b21f5519bd
Signed-off-by: nickboldt <nboldt@redhat.com>
@nickboldt
Copy link
Contributor Author

BIggest change in this set is moving from Dockerfile.rhel (uses alpine + quay.io/openshiftio/rhel-base-httpd:latest) to rhel.Dockerfile (uses images from RHCC: ubi8-minimal and registry.access.redhat.com/rhscl/httpd-24-rhel7:2.4-104)

…iles

Change-Id: I2b97cd814980d336b0b788e5526679d798ae2481
Signed-off-by: nickboldt <nboldt@redhat.com>
Dockerfile Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
rhel.Dockerfile Outdated Show resolved Hide resolved
rhel.Dockerfile Outdated Show resolved Hide resolved
scripts/cache_artifacts.sh Show resolved Hide resolved
rhel.Dockerfile Show resolved Hide resolved
@benoitf
Copy link
Contributor

benoitf commented Sep 30, 2019

why rhel.Dockerfile instead of Dockerfile.rhel ? it's hard to find it also it's not consistent with eclipse-che dockerfiles https://github.com/eclipse/che/tree/master/dockerfiles/che

rhel.Dockerfile Outdated Show resolved Hide resolved
@nickboldt
Copy link
Contributor Author

why rhel.Dockerfile instead of Dockerfile.rhel

the Dockerfile file extension is registered automatically in VSCode. The Dockerfile.* is not. This is why these files get autocompletion and linting:

  • Dockerfile
  • .Dockerfile
  • rhel.Dockerfile

And this does not:

  • Dockerfile.rhel

So to avoid bugs in the rhel version (which people are NOT using in upstream anyway, and aren't being run in CI) I'd rather at least make it easier to edit them :)

rhel.Dockerfile Outdated Show resolved Hide resolved
…ded for Brew and the simpler '/usr/bin/pip3.6 install yq jsonschema' approach for online users; remove unneeded port, fix entry point

Change-Id: I0c356dd93ebc714fc7534e86d8a398b97463c04d
Signed-off-by: nickboldt <nboldt@redhat.com>
… .nix default paths)

Change-Id: I5e9e15e955403b0fc1ffb97d28b1e1274673b883
Signed-off-by: nickboldt <nboldt@redhat.com>
…starts for @amisvek's advice

Change-Id: I0d8c89776c3dcf30080dd43823b40c49cd01be60
Signed-off-by: nickboldt <nboldt@redhat.com>
Change-Id: I6ae8525d958eddbefa254fa371e93ed580dc869b
Signed-off-by: nickboldt <nboldt@redhat.com>
…what doesn't work in downstream

Change-Id: Ia1825e554135a7ca3463dbc27252b9ef9908cba3
Signed-off-by: nickboldt <nboldt@redhat.com>
@nickboldt
Copy link
Contributor Author

two remaining questions are:

• about the .repo file (and reorging the gh repo itself to nest stuff under build/dockerfiles/ - which seems like a low-value change) and
• the question about a simple RUN rm -f ... vs. adding a --latest-only flag to the index.sh script so it can ignore non-latest content when enerating index.json

…g the meta.yaml too (OMG thanks @amisvek for the quick fix)

Change-Id: If5ef526355a9c358af7b4a0c4ad917b1bc9528a8
Signed-off-by: nickboldt <nboldt@redhat.com>
@benoitf
Copy link
Contributor

benoitf commented Oct 1, 2019

about Docker.rhel and rhel.Docker pattern I think we should not be driven by a VS Code extension.

also you've a way to make it working
microsoft/vscode-docker#192

Copy link
Contributor

@amisevsk amisevsk left a comment

Choose a reason for hiding this comment

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

Approving with some suggestions.

fi && \
ln -s /usr/bin/python3.6 /usr/bin/python && \
# test install worked
for d in python yq jq jsonschema; do echo -n "$d: "; $d --version; done
Copy link
Contributor

Choose a reason for hiding this comment

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

In the eventual reorg, this could be pulled into a script that we copy in during the build to keep the dockerfile simpler.


# BEGIN these steps might not be required
RUN sed -i /etc/httpd/conf/httpd.conf \
-e "s,Listen 80,Listen 8080," \
Copy link
Contributor

Choose a reason for hiding this comment

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

s,Listen 80,Listen 8080, is not required as the default is Listen 0.0.0.0:8080 (so there aren't matches), but the other lines do make changes (still don't know if they're necessary).

-e "s,logs/error_log,/dev/stderr," \
-e "s,logs/access_log,/dev/stdout," \
-e "s,AllowOverride None,AllowOverride All," && \
chmod a+rwX /etc/httpd/conf /run/httpd
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe these dirs are already access mode 777.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So you want me to remove this? or add -c to see what actually happens, if anything?

rhel.Dockerfile Outdated Show resolved Hide resolved
scripts/uid_entrypoint.sh Outdated Show resolved Hide resolved
Change-Id: Ic897685dd0873fc5d48e775fe15fd9ca4a55ddad
Signed-off-by: nickboldt <nboldt@redhat.com>
RUN chmod -R g+rwX /build

# To only cache files from /latest/ folders, use ./cache_artifacts.sh v3 --latest-only
# and uncomment line above to remove files so they're not included in index.json -- RUN rm -fr $(find /build/v3 -name 'meta.yaml' | grep -v "/latest/" | grep -o ".*/")
Copy link
Contributor

Choose a reason for hiding this comment

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

For the non-rhel dockerfile, this could be implemented as an ARG (ONLY_LATEST=true) to avoid having to edit the dockerfile for specific builds.

Change-Id: I64530e5a5cca77bbf2aabefc140bb1b6d8efe859
Signed-off-by: nickboldt <nboldt@redhat.com>
@@ -0,0 +1,129 @@
#
Copy link
Contributor

Choose a reason for hiding this comment

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

Still a mistake to me to have rhel.Dockerfile instead of Dockerfile.rhel. It spread related files to different locations

@@ -0,0 +1,5 @@
[epel-7]
Copy link
Contributor

Choose a reason for hiding this comment

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

this file should be suffixes/prefixed by rhel to know it's only for rhel

@@ -0,0 +1,6 @@
#!/bin/bash -x
Copy link
Contributor

Choose a reason for hiding this comment

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

It i missing copyright headers

@@ -0,0 +1,7 @@
#!/bin/bash
if ! whoami &> /dev/null; then
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing copyright headers

#!/bin/bash
if ! whoami &> /dev/null; then
if [ -w /etc/passwd ]; then
echo "${USER_NAME:-jboss}:x:$(id -u):0:${USER_NAME:-jboss} user:${HOME}:/sbin/nologin" >> /etc/passwd
Copy link
Contributor

Choose a reason for hiding this comment

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

Why jboss ?

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