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

Added docker labels - follows label schema namespace #94

Merged
merged 4 commits into from Oct 2, 2017

Conversation

Projects
None yet
3 participants
@waseem18
Contributor

waseem18 commented Oct 1, 2017

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 referenced this pull request Oct 1, 2017

Closed

Add label to Dockerfile #446

@vharsh vharsh requested a review from kmova Oct 1, 2017

@codecov

This comment has been minimized.

Show comment
Hide comment
@codecov

codecov bot Oct 1, 2017

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.

codecov bot commented Oct 1, 2017

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

This comment has been minimized.

Show comment
Hide comment
@kmova

kmova Oct 1, 2017

Member

@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.?
Member

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

This comment has been minimized.

Show comment
Hide comment
@waseem18

waseem18 Oct 1, 2017

Contributor

@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!

Contributor

waseem18 commented Oct 1, 2017

@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

This comment has been minimized.

Show comment
Hide comment
@kmova

kmova Oct 1, 2017

Member

@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"
Member

kmova commented Oct 1, 2017

@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

This comment has been minimized.

Show comment
Hide comment
@waseem18

waseem18 Oct 1, 2017

Contributor

Sure! I'll make the required changes

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

Contributor

waseem18 commented Oct 1, 2017

Sure! I'll make the required changes

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

@waseem18

This comment has been minimized.

Show comment
Hide comment
@waseem18

waseem18 Oct 1, 2017

Contributor

Made the label changes @kmova

Contributor

waseem18 commented Oct 1, 2017

Made the label changes @kmova

@kmova

kmova approved these changes Oct 2, 2017

good work @waseem18

@waseem18

This comment has been minimized.

Show comment
Hide comment
@waseem18

waseem18 Oct 2, 2017

Contributor

Thanks @kmova

Contributor

waseem18 commented Oct 2, 2017

Thanks @kmova

@vharsh vharsh merged commit a206f30 into openebs:master Oct 2, 2017

3 checks passed

codecov/patch Coverage not affected when comparing 1ea0386...e1e439e
Details
codecov/project 12.36% remains the same compared to 1ea0386
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@vharsh

This comment has been minimized.

Show comment
Hide comment
@vharsh
Member

vharsh commented Oct 2, 2017

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