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.

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

Jump to bottom

Added docker labels - follows label schema namespace #94

Merged
merged 4 commits into from Oct 2, 2017
Merged

Added docker labels - follows label schema namespace #94

merged 4 commits into from Oct 2, 2017

Conversation

waseem18
Copy link
Contributor

@waseem18 waseem18 commented Oct 1, 2017 •
edited

This PR adds labels to docker file. The changes follow the label schema namespace.

Addresses the issue discussed at openebs/openebs#446 (comment):

@vharsh Let me know your review.

How to run : docker build --build-arg VERSION=0.4 --build-arg BUILD_DATE=2017-08-01T23:20:50.52Z .

@waseem18 waseem18 mentioned this pull request Oct 1, 2017
Closed
@vharsh vharsh requested a review from kmova October 1, 2017 15:33
@codecov
Copy link

codecov bot commented Oct 1, 2017 •
edited

Codecov Report

Merging #94 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #94   +/-   ##
=======================================
  Coverage   12.36%   12.36%           
=======================================
  Files          42       42           
  Lines        5954     5954           
=======================================
  Hits          736      736           
  Misses       5175     5175           
  Partials       43       43

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1ea0386...e1e439e. Read the comment docs.

kmova
kmova reviewed Oct 1, 2017
View reviewed changes
buildscripts/docker/Dockerfile Outdated
ARG VERSION
ARG BUILD_DATE

RUN echo $BUILD_DATE
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this echo required?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh! Sorry for that. I was just testing stuff and it got in. Will remove it

@kmova
Copy link
Contributor

kmova commented Oct 1, 2017

@waseem18 -- please help me with following queries:

  • what will be the impact if the build args are not provided to the docker build command?
  • Can you provide the output of the command -- docker inspect , on image created with and without passing the build args.?

@waseem18
Copy link
Contributor Author

waseem18 commented Oct 1, 2017 •
edited

@kmova When the build args are not provided, the build fails with the error One or more build-args [VERSION] were not consumed, failing build.

I just found that, even if you don't specify the ARG's docker builds the image. One thing is that we can provide a default value to each argument in Dockerfile so whenever ARG's are not specified, these default values gets accepted.

We need to research a bit if we want the build to fail when the ARG's are not specified!

@kmova
Copy link
Contributor

kmova commented Oct 1, 2017 •
edited

@waseem18

Just looking at the labels.. version and build-date are good.

Remove vendor altogether. "OpenEBS is an OpenSource Project"..

Let us add the following additional labels:

org.label-schema.name="maya"
org.label-schema.description="CLI for OpenEBS"
org.label-schema.url="http://www.openebs.io/"
org.label-schema.vcs-url="https://github.com/openebs/maya"
org.label-schema.schema-version="1.0"

@waseem18
Copy link
Contributor Author

waseem18 commented Oct 1, 2017 •
edited

Sure! I'll make the required changes

I've updated my previous comment @kmova
Kindly look at it

@waseem18
Copy link
Contributor Author

waseem18 commented Oct 1, 2017

Made the label changes @kmova

vharsh
vharsh reviewed Oct 1, 2017
View reviewed changes
buildscripts/docker/Dockerfile Outdated
LABEL org.label-schema.description="CLI for OpenEBS"
LABEL org.label-schema.url="http://www.openebs.io/"
LABEL org.label-schema.vcs-url="https://github.com/openebs/maya"
org.label-schema.schema-version="1.0"
Copy link
Contributor

@vharsh vharsh Oct 1, 2017 •
edited

Choose a reason for hiding this comment

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

LABEL missing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added @vharsh

kmova
kmova approved these changes Oct 2, 2017
View reviewed changes
Copy link
Contributor

@kmova kmova left a comment

Choose a reason for hiding this comment

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

good work @waseem18

@waseem18
Copy link
Contributor Author

waseem18 commented Oct 2, 2017

Thanks @kmova

@vharsh vharsh merged commit a206f30 into openebs-archive:master Oct 2, 2017
@vharsh
Copy link
Contributor

vharsh commented Oct 2, 2017

fixes openebs/openebs#446

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

@vharsh vharsh vharsh left review comments

@kmova kmova kmova approved these changes

Assignees

@waseem18 waseem18

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@waseem18 @kmova @vharsh