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

UPSTREAM: 34020: Allow empty annotation values #11210

Conversation

juanvallejo
Copy link
Contributor

@juanvallejo juanvallejo commented Oct 4, 2016

UPSTREAM: kubernetes/kubernetes#34020

Fixes:
#11175

Annotations with empty values can be used, for example, in oadm
diagnostics logging. This patch removes the client-side check for empty
values in an annotation key-value pair.

Before

$ oc annotate pod zookeeper-1 openshift.io/node-selector="" --overwrite
error: invalid annotation format: openshift.io/node-selector=

After

$ oc annotate pod zookeeper-1 openshift.io/node-selector="" --overwrite
pod "zookeper-1" annotated
$ oc get po/zookeeper-1 --template='{{.metadata.annotations}}'
map[... openshift.io/node-selector: openshift.io/scc:anyuid test-label:test]

@openshift/cli-review

@liggitt
Copy link
Contributor

liggitt commented Oct 4, 2016

needs upstream

@juanvallejo juanvallejo changed the title Allow empty annotation values UPSTREAM: 34020: Allow empty annotation values Oct 4, 2016
@juanvallejo juanvallejo force-pushed the jvallejo/allow-empty-annotation-values branch 4 times, most recently from c242a8b to 4faa06a Compare October 5, 2016 14:20
Annotations with empty values can be used, for example, in diagnostics
logging. This patch removes the client-side check for empty values in
an annotation key-value pair.

**Before**
```
$ kubectl annotate pod zookeeper-1 node-selector="" --overwrite
error: invalid annotation format: node-selector=
```

**After**
```
$ kubectl annotate pod zookeeper-1 node-selector="" --overwrite
pod "zookeper-1" annotated
```

```
$ kubectl get po/zookeeper-1 --template='{{.metadata.annotations}}'
map[... node-selector: test-label:test]
```
@juanvallejo juanvallejo force-pushed the jvallejo/allow-empty-annotation-values branch from d32a9d8 to 751d01c Compare October 5, 2016 17:25
@fabianofranz
Copy link
Member

Does this work without quotes?

oc annotate pod zookeeper-1 openshift.io/node-selector= --overwrite

@juanvallejo
Copy link
Contributor Author

juanvallejo commented Oct 5, 2016

@fabianofranz

Does this work without quotes?

Yeah, just checked both oc label and oc annotate and they both work without labels, as long as the = is present after the key

@fabianofranz
Copy link
Member

LGTM [merge]

@openshift-bot
Copy link
Contributor

[Test]ing while waiting on the merge queue

@juanvallejo juanvallejo force-pushed the jvallejo/allow-empty-annotation-values branch from 751d01c to 94b23f9 Compare October 10, 2016 19:05
@juanvallejo juanvallejo force-pushed the jvallejo/allow-empty-annotation-values branch from 94b23f9 to cd48fb0 Compare October 10, 2016 19:46
@openshift-bot
Copy link
Contributor

Evaluated for origin test up to cd48fb0

@fabianofranz
Copy link
Member

re[merge]

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/9841/)

@juanvallejo
Copy link
Contributor Author

integration test flaked on #11240

@fabianofranz
Copy link
Member

flaked on #11240 re[merge]

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to cd48fb0

@openshift-bot
Copy link
Contributor

openshift-bot commented Oct 18, 2016

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/10167/) (Base Commit: 3ed3b35) (Image: devenv-rhel7_5194)

@openshift-bot openshift-bot merged commit f6bd74b into openshift:master Oct 18, 2016
@juanvallejo juanvallejo deleted the jvallejo/allow-empty-annotation-values branch October 18, 2016 13:46
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.

4 participants