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

Docker Image Metadata proposal #906

Merged
merged 1 commit into from
May 6, 2015

Conversation

mfojtik
Copy link
Contributor

@mfojtik mfojtik commented Feb 5, 2015

No description provided.

@mfojtik
Copy link
Contributor Author

mfojtik commented Feb 5, 2015

@pmorie @smarterclayton @bparees @csrwng

This is very rough version but all comments appreciated ;-)


## The list of proposed metadata

### ACCEPT_TAR_STREAM
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Per discussion with @smarterclayton this will likely be changed to TAR_ON_INPUT or SOURCE_ON_INPUT and there will be a Trello card to support this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@smarterclayton @bparees so I think this might not be needed here after all. Since there are no images that I know if that support streaming application source as a tar to entrypoint, we can omit this metadata.

We were talking today about detecting ONBUILD instructions, which is already available in docker inspect output, so no special flag needed for that as well.

@mfojtik
Copy link
Contributor Author

mfojtik commented Feb 5, 2015

@ncdc rocks! thanks, I fixed them all.

somebody uses my image in OpenShift v3, the BuildConfig generated for this image
will default the build strategy to STI. For this, I only need to specify if my
entrypoint accepts a 'tar' stream that contains the application as an input.

Copy link
Contributor

Choose a reason for hiding this comment

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

application source?

@ryanj
Copy link

ryanj commented Feb 5, 2015

Will this metadata file (or a future implementation of it), be stored in a standard location inside my project repo?

As a solution packager, I'd want to declare external dependencies using a standard filename, location, and schema. I'd want to bundle this info (or a subset of it) inside my repo, to use as the basis for a template file.

@bparees
Copy link
Contributor

bparees commented Feb 5, 2015

@ryanj this is image metadata, not project metadata, and it's not a file, just ENV variables defined on the image.

distinguish between them, the proposed format for the value us following:

```
SUBSCRIBES="service:redis/6379,image:openshift/redis"
Copy link
Contributor

Choose a reason for hiding this comment

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

it bothers me that the service format here (redis/6379) does not match the publish format (redis/tcp/6379). I think we should make them match.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was thinking to make 'tcp' default (less writing)

Copy link
Contributor

Choose a reason for hiding this comment

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

that's fine, but you should do that in the publish format too then. the syntax and behavior of both env variables should be the same, i think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bparees yes, will update publishes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bparees actually it is:

This variable contains a comma separated list of services that the Docker image
publishes. The service is described as <name>/PROTO/PORT (eg.
mysql/tcp/3128) where the 'PROTO' part is optional and default to 'tcp'.

Copy link
Contributor

Choose a reason for hiding this comment

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

having the protocol be optional and in the middle of the stanza is going to make parsing harder/typos easier, might be better to put the optional protocol at the end (mysql/3128/tcp).

that way someone can't accidentally supply a value like "tcp/3128" which we'd consider valid

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't like subscribes as a term. It's really a soft dependency on another component.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@smarterclayton how about SUGGESTS ?

@mfojtik mfojtik force-pushed the metadata_proposal branch 4 times, most recently from d3cbfa0 to a039318 Compare April 28, 2015 09:02
@mfojtik
Copy link
Contributor Author

mfojtik commented Apr 28, 2015

@smarterclayton the Docker guidelines about labels says that the dashes are ok:

Keys should only consist of lower-cased alphanumeric characters, dots and dashes (for example, [a-z0-9-.])

I reworked this proposal to also include displayName, added notion about namespaces and polished the text to be LABEL friendly.

Can you please take a look? Also I'm waiting now how the discussion about LABEL standards ends up. However, in the mean time, do you think we can merge this and switch our Docker image to use the LABELS we proposed here? I don't want to do this as last-minute change as this might break people demos, web console, imageStream imports, etc...

@mfojtik mfojtik force-pushed the metadata_proposal branch 3 times, most recently from 14e71dc to 9db337a Compare April 28, 2015 14:12
[`expose-services`](#expose-services) | []string | openshift.io
[`non-scale`](#non-scale) | bool | openshift.io
[`min-cpu`](#min-cpu) | string | openshift.io(?)
[`min-memory`](#min-memory) | string | openshift.io(?)
Copy link
Contributor

Choose a reason for hiding this comment

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

i'd expect most of these, except maybe tags and wants, to be in the k8s.io namespace.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bparees that is why I put question mark on the last two (they are post-3.0 anyway)

For 'non-scale' and 'expose-services' I'm not sure. The nonScale will be primarily used in OpenShift UI to indicate that this image cannot be scaled with replication controller (so the number of replicas should be 1). As we discussed before, this will be only suggestion as nothing prevents you to scale it up....

For expose-services, yeah, that might end up as k8s.io namespace if Kubernetes is going to pick it up and use it for something. @smarterclayton do you know if there are any plans on kube side to use this metadata? (like to setup livenessprobe?)

Copy link
Contributor

Choose a reason for hiding this comment

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

On Apr 29, 2015, at 4:35 AM, Michal Fojtik notifications@github.com wrote:

In docs/proposals/metadata.md:

+Kubernetes the namespace is k8s.io/. For simple labels, like displayName or
+description there might be no namespace set if they end up as standard in
+Docker.
+
+## Image Metadata
+
+Name | Type | Target Namespace |
+--------------------------------------|--------- | ------------------
+tags | []string | openshift.io
+wants | []string | openshift.io
+display-name | string | k8s.io
+description | string | k8s.io
+expose-services | []string | openshift.io
+non-scale | bool | openshift.io
+min-cpu | string | openshift.io(?)
+min-memory | string | openshift.io(?)
@bparees that is why I put question mark on the last two (they are post-3.0 anyway)

For 'non-scale' and 'expose-services' I'm not sure. The nonScale will be primarily used in OpenShift UI to indicate that this image cannot be scaled with replication controller (so the number of replicas should be 1). As we discussed before, this will be only suggestion as nothing prevents you to scale it up....

For expose-services, yeah, that might end up as k8s.io namespace if Kubernetes is going to pick it up and use it for something. @smarterclayton do you know if there are any plans on kube side to use this metadata? (like to setup livenessprobe?)

Maybe not as it currently is, but with some modification. We don't have a lot of bandwidth available upstream for discussing it though until the other issues are closed. We can always change post 3.0


Reply to this email directly or view it on GitHub.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@smarterclayton so can we go ahead and start changing our Docker images now? (and maybe merge this veteran PR? ;-)

Copy link
Contributor

Choose a reason for hiding this comment

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

what happens when a docker 1.5 client pulls an image with labels?

also we can't remove the existing ENV "labels" until we update the openshift code to look at the docker labels instead of the ENV variables, so this probably needs to be a 3 step process. (and we can't do it until we've moved to a 1.6 prereq)

Copy link
Contributor

Choose a reason for hiding this comment

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

I assume 1.5 won't see the labels.

We require 1.6 now (v2 registry PR merged). Unfortunately we're still fixing some build/release issues, but 1.6 is now a hard minimum requirement.

@bparees
Copy link
Contributor

bparees commented Apr 29, 2015

@mfojtik one minor discussion point, otherwise i like this and can't wait to move to docker labels instead of env variables :)

## Use cases

1. As an author of Docker image that is going to be consumed by the OpenShift
platform, I want to express what categories/tags my Docker image will belongs
Copy link
Member

Choose a reason for hiding this comment

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

will belongs to

@mfojtik
Copy link
Contributor Author

mfojtik commented May 6, 2015

@soltysh @bparees @smarterclayton fixed last comments, I going to merge this now (we can update later if necessary), so I can reference to this in other docs.

[merge]

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_origin/1822/) (Image: devenv-fedora_1450)

@openshift-bot
Copy link
Contributor

[Test]ing while waiting on the merge queue

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin/2033/)

@openshift-bot
Copy link
Contributor

Evaluated for origin up to f3711ec

openshift-bot pushed a commit that referenced this pull request May 6, 2015
@openshift-bot openshift-bot merged commit 5fef529 into openshift:master May 6, 2015
@mfojtik mfojtik deleted the metadata_proposal branch August 25, 2015 09:39
jpeeler added a commit to jpeeler/origin that referenced this pull request Jun 15, 2017
…service-catalog/' changes from c91fecb..1bfff53

1bfff53 instance never provisioned should just delete (openshift#891)
1ae26db Adding a fake broker server (openshift#928)
6403076 docs: fix quoting issue, clarify naming in auth.md (openshift#931)
8ac0775 Merge branch 'pr/927'
02af952 Merge branch 'pr/876'
2aa84f9 add Jenkins badge to README
0c08788 Brokers must have at least one service (openshift#930)
cbfa39b Add PodPreset support (openshift#917)
0d9b810 refactor Jenkins GitHub status postback to work on non-PR commits (openshift#916)
066159d Converting the AuthSecret field to a union AuthInfo type (openshift#877)
203af5c Add leader election namespace configuration (openshift#920)
5831502 Add example JSON schema to controller unit tests (openshift#918)
b78ab99 Fix usage of finalizers (openshift#894)
d3d29f0 Enable pprof in controller-manager (openshift#896)
f4233a0 Correct parameter schema support (openshift#912)
05c6f00 bump image tags from v0.0.8 to v0.0.9 (openshift#910)
97d278a Add support for OSB parameter schemas (openshift#822)
3e4120e Fix nil dereference panic on request timeout (openshift#906)
d8c7494 Add feature gate for audit options in helm chart (openshift#904)
89ce1cd Decompose controller unit tests (openshift#899)
a1e83b2 Add e2e for walkthrough (openshift#832)
4679685 Add support for audit log options (openshift#897)
262a94f Do not allow updates to an object if asynchronous operation is in progress (openshift#853)
7295dad Validate that a ServiceClass must have at least one plan (openshift#879)
9db9fa4 Decompose controller.go (openshift#893)
c3ea9bd Nits in our types (openshift#854)
1d8280a bump tags from v0.0.7 to v0.0.8 (openshift#892)
5e6925d Clean up the OSB client (openshift#888)
fe6aee9 cleaning up logs and adding more log detail (openshift#874)
f41516f Detect if a TPR update represents a soft delete (openshift#836)
9ce99f3 Add functions on Makefile for build and tag
REVERT: c91fecb Merge pull request openshift#1 from jpeeler/origin-build
REVERT: 55ccf3d origin build: add _output to .gitignore
REVERT: 8352e14 origin build: make build-go and build-cross work
REVERT: d969641 origin build: modify hard coded path
REVERT: 30000cc origin build: add origin tooling

git-subtree-dir: cmd/service-catalog/go/src/github.com/kubernetes-incubator/service-catalog
git-subtree-split: 1bfff53
jpeeler added a commit to jpeeler/origin that referenced this pull request Jun 15, 2017
…service-catalog/' changes from c91fecb..568a7b9

568a7b9 origin build: add origin tooling
1bfff53 instance never provisioned should just delete (openshift#891)
1ae26db Adding a fake broker server (openshift#928)
6403076 docs: fix quoting issue, clarify naming in auth.md (openshift#931)
8ac0775 Merge branch 'pr/927'
02af952 Merge branch 'pr/876'
2aa84f9 add Jenkins badge to README
0c08788 Brokers must have at least one service (openshift#930)
cbfa39b Add PodPreset support (openshift#917)
0d9b810 refactor Jenkins GitHub status postback to work on non-PR commits (openshift#916)
066159d Converting the AuthSecret field to a union AuthInfo type (openshift#877)
203af5c Add leader election namespace configuration (openshift#920)
5831502 Add example JSON schema to controller unit tests (openshift#918)
b78ab99 Fix usage of finalizers (openshift#894)
d3d29f0 Enable pprof in controller-manager (openshift#896)
f4233a0 Correct parameter schema support (openshift#912)
05c6f00 bump image tags from v0.0.8 to v0.0.9 (openshift#910)
97d278a Add support for OSB parameter schemas (openshift#822)
3e4120e Fix nil dereference panic on request timeout (openshift#906)
d8c7494 Add feature gate for audit options in helm chart (openshift#904)
89ce1cd Decompose controller unit tests (openshift#899)
a1e83b2 Add e2e for walkthrough (openshift#832)
4679685 Add support for audit log options (openshift#897)
262a94f Do not allow updates to an object if asynchronous operation is in progress (openshift#853)
7295dad Validate that a ServiceClass must have at least one plan (openshift#879)
9db9fa4 Decompose controller.go (openshift#893)
c3ea9bd Nits in our types (openshift#854)
1d8280a bump tags from v0.0.7 to v0.0.8 (openshift#892)
5e6925d Clean up the OSB client (openshift#888)
fe6aee9 cleaning up logs and adding more log detail (openshift#874)
f41516f Detect if a TPR update represents a soft delete (openshift#836)
9ce99f3 Add functions on Makefile for build and tag
REVERT: c91fecb Merge pull request openshift#1 from jpeeler/origin-build
REVERT: 55ccf3d origin build: add _output to .gitignore
REVERT: 8352e14 origin build: make build-go and build-cross work
REVERT: d969641 origin build: modify hard coded path
REVERT: 30000cc origin build: add origin tooling

git-subtree-dir: cmd/service-catalog/go/src/github.com/kubernetes-incubator/service-catalog
git-subtree-split: 568a7b9dbdc4fdd1fabffdd52af030ec73124b89
jpeeler pushed a commit to jpeeler/origin that referenced this pull request Feb 1, 2018
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

7 participants