Skip to content
This repository has been archived by the owner on Jan 19, 2018. It is now read-only.

Add openshift LABELs #334

Merged
merged 2 commits into from
Oct 19, 2015
Merged

Add openshift LABELs #334

merged 2 commits into from
Oct 19, 2015

Conversation

aweiteka
Copy link
Contributor

In order for the OpenShift provider to natively
support atomicapp we need to pass in a flag that
identifies the image as an openshift self-executing
job. We also have to instruct OpenShift to pass
in user auth token so we can run oc as that user.

cc @smarterclayton to confirm LABEL correct key=value

In order for the OpenShift provider to natively
support atomicapp we need to pass in a flag that
identifies the image as an openshift self-executing
job. We also have to instruct OpenShift to pass
in user auth token so we can run oc as that user.
@aweiteka aweiteka added this to the CDK 2 beta-3 milestone Oct 16, 2015
@dustymabe
Copy link
Contributor

A couple of thoughts here. I'd actually like the labels to not be specific to openshift in case other providers would like to benefit from these labels in a similar way. Perhaps we can transform them into something like:

io.openshift.generate.job=true --> io.projectatomic.nulecule=true
io.openshift.generate.token.as=env:TOKEN_ENV_VAR --> io.projectatomic.nulecule.authtoken=env:AUTHTOKEN_ENV_VAR

My new label proposals may not make sense but maybe we can come up with something that isn't openshift specific that does? @smarterclayton @aweiteka what do you think?

@smarterclayton
Copy link

The point of the labels being specific to openshift is that they are only used in one context - the OpenShift new-app generator. In the future, making these labels generic is great - but for that, there needs to be use of them. We can always add more labels, but I'm generally not in support of making generic things and hoping people use them. I want to make concrete things and prove their utility. I would prefer these to be openshift specific for now.

@landscape-bot
Copy link

Code Health
Code quality remained the same when pulling 89185b8 on aweiteka:openshift-label into 14e7499 on projectatomic:master.

@dustymabe
Copy link
Contributor

Fair enough. I just don't want to deal with migration issues in the future if we decide to move to a more generic labeling scheme. I guess since we have a heavy influence in the openshift community it would be easier to change there than if we were working with some community where we didn't have influence.

@goern
Copy link
Contributor

goern commented Oct 19, 2015

On the other hand, a LABEL is just a label, no one will hinder some 3rd party platform/provider/... to evaluate a label called io.openshift...

@goern
Copy link
Contributor

goern commented Oct 19, 2015

doc and test (plan) are missing, otherwise LGTM

@aweiteka
Copy link
Contributor Author

@goern docs added. Test plan will be part of the code change PR that addresses #321 that enables openshift-native running of atomic app.

@landscape-bot
Copy link

Code Health
Code quality remained the same when pulling 154a48a on aweiteka:openshift-label into 14e7499 on projectatomic:master.

goern added a commit that referenced this pull request Oct 19, 2015
Add OpenShift LABELs to enable native Atomic App support
@goern goern merged commit 5790fc5 into projectatomic:master Oct 19, 2015
@smarterclayton
Copy link

Initial code is here openshift/origin#5378

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants