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: 30313: remove duplicate errors from aggregate error outputs #10317

Conversation

juanvallejo
Copy link
Contributor

@juanvallejo juanvallejo commented Aug 9, 2016

Related BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1257800
UPSTREAM: kubernetes/kubernetes#30313

Duplicate error messages are sometimes shown when displaying aggregate
errors:

$ oc label pod/database-1-fn0r7 qwer1345%$$#=self

* metadata.labels: Invalid value: "qwer1345%5602#": name part must match
* the regex ([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9] (e.g. 'MyName' or
* 'my.name' or '123-abc')
* metadata.labels: Invalid value: "qwer1345%5602#": name part must match
* the regex ([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9] (e.g. 'MyName' or
* 'my.name' or '123-abc')
* metadata.labels: Invalid value: "qwer1345%5602#": name part must match
* the regex ([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9] (e.g. 'MyName' or
* 'my.name' or '123-abc')

This patch removes any duplicate messages (adjacent or not) that appear
in the final list of errors.

Before

$ oc label pod/database-1-fn0r7 qwer1345%$$#=self

The Pod "database-1-fn0r7" is invalid.

* metadata.labels: Invalid value: "qwer1345%24704#": name part must match the regex ([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9] (e.g. 'MyName' or 'my.name' or '123-abc')
* metadata.labels: Invalid value: "qwer1345%24704#": name part must match the regex ([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9] (e.g. 'MyName' or 'my.name' or '123-abc')
* metadata.labels: Invalid value: "qwer1345%24704#": name part must match the regex ([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9] (e.g. 'MyName' or 'my.name' or '123-abc')
➜  origin git:(jvallejo_remove-duplicate-err-oclabel-ocenv) ✗ oc label pod/database-1-fn0r7 qwer1345%$$#=self
After

$ oc label pod/database-1-fn0r7 qwer1345%$$#=self

The Pod "database-1-fn0r7" is invalid.
metadata.labels: Invalid value: "qwer1345%24704#": name part must match the regex ([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9] (e.g. 'MyName' or 'my.name' or '123-abc')

cc @fabianofranz

@juanvallejo
Copy link
Contributor Author

[test]

@ncdc ncdc changed the title UPSTREAM: 30313: remove duplicate errors from aggregate error outputs [DO NOT MERGE] UPSTREAM: 30313: remove duplicate errors from aggregate error outputs Aug 10, 2016
@ncdc
Copy link
Contributor

ncdc commented Aug 10, 2016

Do not merge until upstream has merged

@juanvallejo juanvallejo force-pushed the jvallejo_remove-duplicate-err-oclabel-ocenv branch from 1ba3bfd to d5d5046 Compare August 11, 2016 19:26
@fabianofranz
Copy link
Member

fabianofranz commented Aug 12, 2016

LGTM if this only suppress a message if they are exactly equal when shown to the user. Sometimes we have a given error message shown multiple times to user (in oc new-app for example) but it takes the resource type/name as a prefix for the message. In that case, it still has to be shown.

error: DeploymentConfig "httpd" is invalid: [metadata.labels: Invalid value: "*(%$^$%": must match the regex (([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])? (e.g. 'MyValue' or 'my_value' or '12345'), spec.template.labels: Invalid value: "*(%$^$%": must match the regex (([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])? (e.g. 'MyValue' or 'my_value' or '12345')]
error: Service "httpd" is invalid: [metadata.labels: Invalid value: "*(%$^$%": must match the regex (([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])? (e.g. 'MyValue' or 'my_value' or '12345'), spec.selector: Invalid value: "*(%$^$%": must match the regex (([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])? (e.g. 'MyValue' or 'my_value' or '12345')]

if errorMsgs.Has(sc.Message) {
continue
}
errorMsgs.Insert(sc.Message)
errs[i] = fmt.Errorf("%s: %s", sc.Field, sc.Message)
Copy link
Contributor

Choose a reason for hiding this comment

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

the duplicate check needs to include the field, since that's what the user sees

@juanvallejo
Copy link
Contributor Author

@fabianofranz

Sometimes we have a given error message shown multiple times to user (in oc new-app for example) but it takes the resource type/name as a suffix for the message.

Yeah, this patch only suppresses parts of the error message that repear after <ResourceName> "x" is invalid:, as it was the metadata.labels part that was repeating.

for i, sc := range scs {
// check for duplicate error messages and skip them
if errorMsgs.Has(sc.Message) {
continue
Copy link
Contributor

@liggitt liggitt Aug 12, 2016

Choose a reason for hiding this comment

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

if we ever skip because of this, that means errs has nil errors interspersed, which I would expect to panic when NewAggregate attempts to call Error() on them

Copy link
Contributor

Choose a reason for hiding this comment

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

what you want to do is allocate errs with capacity of len(scs), but length 0, then use errs = append(errs, ...) instead of assigning directly to indexes

Copy link
Member

Choose a reason for hiding this comment

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

Good catch.

@juanvallejo juanvallejo force-pushed the jvallejo_remove-duplicate-err-oclabel-ocenv branch from d5d5046 to a7fed0c Compare August 12, 2016 21:48
@juanvallejo
Copy link
Contributor Author

@liggitt Thanks for the feedback!

what you want to do is allocate errs with capacity of len(scs), but length 0, then use errs = append(errs, ...) instead of assigning directly to indexes

Done!

@juanvallejo juanvallejo force-pushed the jvallejo_remove-duplicate-err-oclabel-ocenv branch 2 times, most recently from 15d970a to 7770899 Compare August 16, 2016 17:10
@juanvallejo
Copy link
Contributor Author

@openshift-bot, the last build failed from the following flakes:

re[test]

@juanvallejo
Copy link
Contributor Author

juanvallejo commented Aug 22, 2016

@ncdc

Do not merge until upstream has merged

Done! kubernetes/kubernetes#30313

@fabianofranz
Copy link
Member

@juanvallejo please remove [DO NOT MERGE] if this is good to go.
@liggitt anything else?

@liggitt
Copy link
Contributor

liggitt commented Aug 22, 2016

one of the commits has a different line count than upstream... is it updated with the merged version from upstream? other than that, looks fine

@juanvallejo juanvallejo changed the title [DO NOT MERGE] UPSTREAM: 30313: remove duplicate errors from aggregate error outputs UPSTREAM: 30313: remove duplicate errors from aggregate error outputs Aug 22, 2016
@juanvallejo
Copy link
Contributor Author

juanvallejo commented Aug 22, 2016

@liggitt

one of the commits has a different line count than upstream... is it updated with the merged version from upstream?

Yes, the changes for this commit and the one from upstream are the same. The files (without this patch) already have different line counts: origin, kubernetes

EDIT: I misunderstood your comment, there is an extra line-break in this commit: 5705847#diff-45d0049c1caf6801fde2ad4e6b3bb545R157

Related BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1257800

Duplicate error messages are sometimes shown when displaying aggregate
errors:

`$ oc label pod/database-1-fn0r7 qwer1345%$$#=self`
```
* metadata.labels: Invalid value: "qwer1345%5602#": name part must match
* the regex ([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9] (e.g. 'MyName' or
* 'my.name' or '123-abc')
* metadata.labels: Invalid value: "qwer1345%5602#": name part must match
* the regex ([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9] (e.g. 'MyName' or
* 'my.name' or '123-abc')
* metadata.labels: Invalid value: "qwer1345%5602#": name part must match
* the regex ([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9] (e.g. 'MyName' or
* 'my.name' or '123-abc')
```

This patch removes any duplicate messages (adjacent or not) that appear
in the final list of errors.
@juanvallejo juanvallejo force-pushed the jvallejo_remove-duplicate-err-oclabel-ocenv branch from 7770899 to 2061aab Compare August 22, 2016 15:50
@fabianofranz
Copy link
Member

@juanvallejo I'd also like a negative kind of positive cmd test. :)

For example in test/cmd/newapp.sh, take the example below and make sure you still get every one of the 3 messages you are expected to get (one for each of ImageStream, DeploymentConfig, and Service).

$ $ oc new-app nginx -l qwer1345%$$#=self
(...)
    error: ImageStream "nginx" is invalid: metadata.labels: Invalid value: "qwer1345%25258#": name part must match (...)
    error: DeploymentConfig "nginx" is invalid: [metadata.labels: Invalid value: "qwer1345%25258#": name part must match (...)
    error: Service "nginx" is invalid: [metadata.labels: Invalid value: "qwer1345%25258#": name part must match (...)

@juanvallejo
Copy link
Contributor Author

@fabianofranz
Done!

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to 29e7025

@openshift-bot
Copy link
Contributor

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

@fabianofranz
Copy link
Member

LGTM [merge]

@openshift-bot
Copy link
Contributor

openshift-bot commented Aug 23, 2016

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/8364/) (Image: devenv-rhel7_4907)

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to 29e7025

@openshift-bot openshift-bot merged commit 1605423 into openshift:master Aug 23, 2016
smarterclayton pushed a commit to smarterclayton/kubernetes that referenced this pull request Sep 1, 2016
Related PR: openshift/origin#10317
Related BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1257800#c3

PR kubernetes#10317 fixed duplicate errors for invalid aggregate errors in
https://github.com/openshift/origin/blob/master/vendor/k8s.io/kubernetes/pkg/kubectl/cmd/util/helpers.go.

However, duplicate aggregate errors that went through
https://github.com/openshift/origin/blob/master/vendor/k8s.io/kubernetes/pkg/util/validation/field/errors.go
were not affected by that patch.

This patch adds duplicate aggregate error checking to
`pkg/util/validation/field/errors.go`

\##### Before
`$ oc set env rc/idling-echo-1 test-abc=1234`
```
error: ReplicationController "idling-echo-1" is invalid:
[spec.template.spec.containers[0].env[0].name: Invalid value:
"test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName", spec.template.spec.containers[0].env[0].name:
Invalid value: "test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"]
```

`$ oc set env rc/node-1 test-abc=1234`
```
error: ReplicationController "idling-echo-1" is invalid:
[spec.template.spec.containers[0].env[0].name: Invalid value:
"test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"]
```

\##### After
`$ oc set env rc/idling-echo-1 test-abc=1234`
```
error: ReplicationController "idling-echo-1" is invalid:
[spec.template.spec.containers[0].env[0].name: Invalid value:
"test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"]
```

`$ oc set env rc/node-1 test-abc=1234`
```
error: ReplicationController "node-1" is invalid:
spec.template.spec.containers[0].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"
```

:100644 100644 203f7cc... 089f21c... M	pkg/util/validation/field/errors.go
jcantrill pushed a commit to jcantrill/origin that referenced this pull request Sep 1, 2016
Related PR: openshift#10317
Related BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1257800#c3

PR openshift#10317 fixed duplicate errors for invalid aggregate errors in
https://github.com/openshift/origin/blob/master/vendor/k8s.io/kubernetes/pkg/kubectl/cmd/util/helpers.go.

However, duplicate aggregate errors that went through
https://github.com/openshift/origin/blob/master/vendor/k8s.io/kubernetes/pkg/util/validation/field/errors.go
were not affected by that patch.

This patch adds duplicate aggregate error checking to
`pkg/util/validation/field/errors.go`

\##### Before
`$ oc set env rc/idling-echo-1 test-abc=1234`
```
error: ReplicationController "idling-echo-1" is invalid:
[spec.template.spec.containers[0].env[0].name: Invalid value:
"test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName", spec.template.spec.containers[0].env[0].name:
Invalid value: "test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"]
```

`$ oc set env rc/node-1 test-abc=1234`
```
error: ReplicationController "idling-echo-1" is invalid:
[spec.template.spec.containers[0].env[0].name: Invalid value:
"test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"]
```

\##### After
`$ oc set env rc/idling-echo-1 test-abc=1234`
```
error: ReplicationController "idling-echo-1" is invalid:
[spec.template.spec.containers[0].env[0].name: Invalid value:
"test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"]
```

`$ oc set env rc/node-1 test-abc=1234`
```
error: ReplicationController "node-1" is invalid:
spec.template.spec.containers[0].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"
```
smarterclayton pushed a commit to smarterclayton/kubernetes that referenced this pull request Sep 2, 2016
Related PR: openshift/origin#10317
Related BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1257800#c3

PR kubernetes#10317 fixed duplicate errors for invalid aggregate errors in
https://github.com/openshift/origin/blob/master/vendor/k8s.io/kubernetes/pkg/kubectl/cmd/util/helpers.go.

However, duplicate aggregate errors that went through
https://github.com/openshift/origin/blob/master/vendor/k8s.io/kubernetes/pkg/util/validation/field/errors.go
were not affected by that patch.

This patch adds duplicate aggregate error checking to
`pkg/util/validation/field/errors.go`

\##### Before
`$ oc set env rc/idling-echo-1 test-abc=1234`
```
error: ReplicationController "idling-echo-1" is invalid:
[spec.template.spec.containers[0].env[0].name: Invalid value:
"test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName", spec.template.spec.containers[0].env[0].name:
Invalid value: "test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"]
```

`$ oc set env rc/node-1 test-abc=1234`
```
error: ReplicationController "idling-echo-1" is invalid:
[spec.template.spec.containers[0].env[0].name: Invalid value:
"test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"]
```

\##### After
`$ oc set env rc/idling-echo-1 test-abc=1234`
```
error: ReplicationController "idling-echo-1" is invalid:
[spec.template.spec.containers[0].env[0].name: Invalid value:
"test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"]
```

`$ oc set env rc/node-1 test-abc=1234`
```
error: ReplicationController "node-1" is invalid:
spec.template.spec.containers[0].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"
```

:100644 100644 203f7cc... 089f21c... M	pkg/util/validation/field/errors.go
smarterclayton pushed a commit to smarterclayton/kubernetes that referenced this pull request Sep 5, 2016
Related PR: openshift/origin#10317
Related BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1257800#c3

PR kubernetes#10317 fixed duplicate errors for invalid aggregate errors in
https://github.com/openshift/origin/blob/master/vendor/k8s.io/kubernetes/pkg/kubectl/cmd/util/helpers.go.

However, duplicate aggregate errors that went through
https://github.com/openshift/origin/blob/master/vendor/k8s.io/kubernetes/pkg/util/validation/field/errors.go
were not affected by that patch.

This patch adds duplicate aggregate error checking to
`pkg/util/validation/field/errors.go`

\##### Before
`$ oc set env rc/idling-echo-1 test-abc=1234`
```
error: ReplicationController "idling-echo-1" is invalid:
[spec.template.spec.containers[0].env[0].name: Invalid value:
"test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName", spec.template.spec.containers[0].env[0].name:
Invalid value: "test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"]
```

`$ oc set env rc/node-1 test-abc=1234`
```
error: ReplicationController "idling-echo-1" is invalid:
[spec.template.spec.containers[0].env[0].name: Invalid value:
"test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"]
```

\##### After
`$ oc set env rc/idling-echo-1 test-abc=1234`
```
error: ReplicationController "idling-echo-1" is invalid:
[spec.template.spec.containers[0].env[0].name: Invalid value:
"test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"]
```

`$ oc set env rc/node-1 test-abc=1234`
```
error: ReplicationController "node-1" is invalid:
spec.template.spec.containers[0].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"
```

:100644 100644 203f7cc... 089f21c... M	pkg/util/validation/field/errors.go
smarterclayton pushed a commit to smarterclayton/kubernetes that referenced this pull request Sep 13, 2016
Related PR: openshift/origin#10317
Related BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1257800#c3

PR kubernetes#10317 fixed duplicate errors for invalid aggregate errors in
https://github.com/openshift/origin/blob/master/vendor/k8s.io/kubernetes/pkg/kubectl/cmd/util/helpers.go.

However, duplicate aggregate errors that went through
https://github.com/openshift/origin/blob/master/vendor/k8s.io/kubernetes/pkg/util/validation/field/errors.go
were not affected by that patch.

This patch adds duplicate aggregate error checking to
`pkg/util/validation/field/errors.go`

\##### Before
`$ oc set env rc/idling-echo-1 test-abc=1234`
```
error: ReplicationController "idling-echo-1" is invalid:
[spec.template.spec.containers[0].env[0].name: Invalid value:
"test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName", spec.template.spec.containers[0].env[0].name:
Invalid value: "test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"]
```

`$ oc set env rc/node-1 test-abc=1234`
```
error: ReplicationController "idling-echo-1" is invalid:
[spec.template.spec.containers[0].env[0].name: Invalid value:
"test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"]
```

\##### After
`$ oc set env rc/idling-echo-1 test-abc=1234`
```
error: ReplicationController "idling-echo-1" is invalid:
[spec.template.spec.containers[0].env[0].name: Invalid value:
"test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"]
```

`$ oc set env rc/node-1 test-abc=1234`
```
error: ReplicationController "node-1" is invalid:
spec.template.spec.containers[0].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"
```

:100644 100644 203f7cc... 089f21c... M	pkg/util/validation/field/errors.go
guilhermebr pushed a commit to guilhermebr/origin that referenced this pull request Sep 17, 2016
Related PR: openshift#10317
Related BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1257800#c3

PR openshift#10317 fixed duplicate errors for invalid aggregate errors in
https://github.com/openshift/origin/blob/master/vendor/k8s.io/kubernetes/pkg/kubectl/cmd/util/helpers.go.

However, duplicate aggregate errors that went through
https://github.com/openshift/origin/blob/master/vendor/k8s.io/kubernetes/pkg/util/validation/field/errors.go
were not affected by that patch.

This patch adds duplicate aggregate error checking to
`pkg/util/validation/field/errors.go`

\##### Before
`$ oc set env rc/idling-echo-1 test-abc=1234`
```
error: ReplicationController "idling-echo-1" is invalid:
[spec.template.spec.containers[0].env[0].name: Invalid value:
"test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName", spec.template.spec.containers[0].env[0].name:
Invalid value: "test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"]
```

`$ oc set env rc/node-1 test-abc=1234`
```
error: ReplicationController "idling-echo-1" is invalid:
[spec.template.spec.containers[0].env[0].name: Invalid value:
"test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"]
```

\##### After
`$ oc set env rc/idling-echo-1 test-abc=1234`
```
error: ReplicationController "idling-echo-1" is invalid:
[spec.template.spec.containers[0].env[0].name: Invalid value:
"test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"]
```

`$ oc set env rc/node-1 test-abc=1234`
```
error: ReplicationController "node-1" is invalid:
spec.template.spec.containers[0].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"
```
smarterclayton pushed a commit to smarterclayton/kubernetes that referenced this pull request Sep 25, 2016
Related PR: openshift/origin#10317
Related BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1257800#c3

PR kubernetes#10317 fixed duplicate errors for invalid aggregate errors in
https://github.com/openshift/origin/blob/master/vendor/k8s.io/kubernetes/pkg/kubectl/cmd/util/helpers.go.

However, duplicate aggregate errors that went through
https://github.com/openshift/origin/blob/master/vendor/k8s.io/kubernetes/pkg/util/validation/field/errors.go
were not affected by that patch.

This patch adds duplicate aggregate error checking to
`pkg/util/validation/field/errors.go`

\##### Before
`$ oc set env rc/idling-echo-1 test-abc=1234`
```
error: ReplicationController "idling-echo-1" is invalid:
[spec.template.spec.containers[0].env[0].name: Invalid value:
"test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName", spec.template.spec.containers[0].env[0].name:
Invalid value: "test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"]
```

`$ oc set env rc/node-1 test-abc=1234`
```
error: ReplicationController "idling-echo-1" is invalid:
[spec.template.spec.containers[0].env[0].name: Invalid value:
"test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"]
```

\##### After
`$ oc set env rc/idling-echo-1 test-abc=1234`
```
error: ReplicationController "idling-echo-1" is invalid:
[spec.template.spec.containers[0].env[0].name: Invalid value:
"test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"]
```

`$ oc set env rc/node-1 test-abc=1234`
```
error: ReplicationController "node-1" is invalid:
spec.template.spec.containers[0].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"
```

:100644 100644 203f7cc... 089f21c... M	pkg/util/validation/field/errors.go
smarterclayton pushed a commit to smarterclayton/kubernetes that referenced this pull request Sep 26, 2016
Related PR: openshift/origin#10317
Related BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1257800#c3

PR kubernetes#10317 fixed duplicate errors for invalid aggregate errors in
https://github.com/openshift/origin/blob/master/vendor/k8s.io/kubernetes/pkg/kubectl/cmd/util/helpers.go.

However, duplicate aggregate errors that went through
https://github.com/openshift/origin/blob/master/vendor/k8s.io/kubernetes/pkg/util/validation/field/errors.go
were not affected by that patch.

This patch adds duplicate aggregate error checking to
`pkg/util/validation/field/errors.go`

\##### Before
`$ oc set env rc/idling-echo-1 test-abc=1234`
```
error: ReplicationController "idling-echo-1" is invalid:
[spec.template.spec.containers[0].env[0].name: Invalid value:
"test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName", spec.template.spec.containers[0].env[0].name:
Invalid value: "test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"]
```

`$ oc set env rc/node-1 test-abc=1234`
```
error: ReplicationController "idling-echo-1" is invalid:
[spec.template.spec.containers[0].env[0].name: Invalid value:
"test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"]
```

\##### After
`$ oc set env rc/idling-echo-1 test-abc=1234`
```
error: ReplicationController "idling-echo-1" is invalid:
[spec.template.spec.containers[0].env[0].name: Invalid value:
"test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"]
```

`$ oc set env rc/node-1 test-abc=1234`
```
error: ReplicationController "node-1" is invalid:
spec.template.spec.containers[0].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"
```

:100644 100644 203f7cc... 089f21c... M	pkg/util/validation/field/errors.go
smarterclayton pushed a commit to smarterclayton/kubernetes that referenced this pull request Oct 25, 2016
Related PR: openshift/origin#10317
Related BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1257800#c3

PR kubernetes#10317 fixed duplicate errors for invalid aggregate errors in
https://github.com/openshift/origin/blob/master/vendor/k8s.io/kubernetes/pkg/kubectl/cmd/util/helpers.go.

However, duplicate aggregate errors that went through
https://github.com/openshift/origin/blob/master/vendor/k8s.io/kubernetes/pkg/util/validation/field/errors.go
were not affected by that patch.

This patch adds duplicate aggregate error checking to
`pkg/util/validation/field/errors.go`

\##### Before
`$ oc set env rc/idling-echo-1 test-abc=1234`
```
error: ReplicationController "idling-echo-1" is invalid:
[spec.template.spec.containers[0].env[0].name: Invalid value:
"test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName", spec.template.spec.containers[0].env[0].name:
Invalid value: "test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"]
```

`$ oc set env rc/node-1 test-abc=1234`
```
error: ReplicationController "idling-echo-1" is invalid:
[spec.template.spec.containers[0].env[0].name: Invalid value:
"test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"]
```

\##### After
`$ oc set env rc/idling-echo-1 test-abc=1234`
```
error: ReplicationController "idling-echo-1" is invalid:
[spec.template.spec.containers[0].env[0].name: Invalid value:
"test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"]
```

`$ oc set env rc/node-1 test-abc=1234`
```
error: ReplicationController "node-1" is invalid:
spec.template.spec.containers[0].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"
```

:100644 100644 203f7cc... 089f21c... M	pkg/util/validation/field/errors.go
ncdc pushed a commit to ncdc/kubernetes that referenced this pull request Dec 22, 2016
Related PR: openshift/origin#10317
Related BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1257800#c3

PR kubernetes#10317 fixed duplicate errors for invalid aggregate errors in
https://github.com/openshift/origin/blob/master/vendor/k8s.io/kubernetes/pkg/kubectl/cmd/util/helpers.go.

However, duplicate aggregate errors that went through
https://github.com/openshift/origin/blob/master/vendor/k8s.io/kubernetes/pkg/util/validation/field/errors.go
were not affected by that patch.

This patch adds duplicate aggregate error checking to
`pkg/util/validation/field/errors.go`

\##### Before
`$ oc set env rc/idling-echo-1 test-abc=1234`
```
error: ReplicationController "idling-echo-1" is invalid:
[spec.template.spec.containers[0].env[0].name: Invalid value:
"test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName", spec.template.spec.containers[0].env[0].name:
Invalid value: "test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"]
```

`$ oc set env rc/node-1 test-abc=1234`
```
error: ReplicationController "idling-echo-1" is invalid:
[spec.template.spec.containers[0].env[0].name: Invalid value:
"test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"]
```

\##### After
`$ oc set env rc/idling-echo-1 test-abc=1234`
```
error: ReplicationController "idling-echo-1" is invalid:
[spec.template.spec.containers[0].env[0].name: Invalid value:
"test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"]
```

`$ oc set env rc/node-1 test-abc=1234`
```
error: ReplicationController "node-1" is invalid:
spec.template.spec.containers[0].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"
```

:100644 100644 203f7cc... 089f21c... M	pkg/util/validation/field/errors.go
soltysh pushed a commit to soltysh/kubernetes that referenced this pull request Jan 12, 2017
Related PR: openshift/origin#10317
Related BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1257800#c3

PR kubernetes#10317 fixed duplicate errors for invalid aggregate errors in
https://github.com/openshift/origin/blob/master/vendor/k8s.io/kubernetes/pkg/kubectl/cmd/util/helpers.go.

However, duplicate aggregate errors that went through
https://github.com/openshift/origin/blob/master/vendor/k8s.io/kubernetes/pkg/util/validation/field/errors.go
were not affected by that patch.

This patch adds duplicate aggregate error checking to
`pkg/util/validation/field/errors.go`

\##### Before
`$ oc set env rc/idling-echo-1 test-abc=1234`
```
error: ReplicationController "idling-echo-1" is invalid:
[spec.template.spec.containers[0].env[0].name: Invalid value:
"test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName", spec.template.spec.containers[0].env[0].name:
Invalid value: "test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"]
```

`$ oc set env rc/node-1 test-abc=1234`
```
error: ReplicationController "idling-echo-1" is invalid:
[spec.template.spec.containers[0].env[0].name: Invalid value:
"test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"]
```

\##### After
`$ oc set env rc/idling-echo-1 test-abc=1234`
```
error: ReplicationController "idling-echo-1" is invalid:
[spec.template.spec.containers[0].env[0].name: Invalid value:
"test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"]
```

`$ oc set env rc/node-1 test-abc=1234`
```
error: ReplicationController "node-1" is invalid:
spec.template.spec.containers[0].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"
```

:100644 100644 203f7cc... 089f21c... M	pkg/util/validation/field/errors.go
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.

5 participants