I discovered this by changing the validation in a way that SHOULD fail
(by allowing something it should not). But it didn't. A different
error happens which totally masks the non-failure I expected. New test
is much more explicit about what failures are expected.
This does not focus on adding test coverage, just making sure the test
is not terrible.
a) Rename the type and drop the constructor
b) Make MatchErrors() into a Test() method
For followup:
c) Consider making ByType() assumed
d) Consider making ByField() assumed and handle nil as "don't care"
e) Consider making ByValue() assumed and handle nil as "don't care"
I fixed up the TestValidateEndpointsCreate path to show the matcher
instead of manual origin checking.
I picked TestValidateTopologySpreadConstraints because it was the last
failing test on my screen when I changed on of the commonly hard-coded
error strings. I fixed exactly those validation errors that were needed
to make this test pass. Some of the Origin values can be debated.
The `field/testing.Matcher` interface allows tests to configure the
criteria by which they want to match expected and actual errors. The
hope is that everyone will use Origin for Invalid errors.
There's some collateral impact for tests which use exact-comparisons and
don't expect origins. These are all candidates for using the matcher.
Update ValidateEndpointsCreate validation tests to use the new Origin field for more precise error comparisons. It leverage the Origin field instead of detailed error messages, improving test robustness and readability.
Co-authored-by: Tim Hockin <thockin@google.com>
The "// import <path>" comment has been superseded by Go modules.
We don't have to remove them, but doing so has some advantages:
- They are used inconsistently, which is confusing.
- We can then also remove the (currently broken) hack/update-vanity-imports.sh.
- Last but not least, it would be a first step towards avoiding the k8s.io domain.
This commit was generated with
sed -i -e 's;^package \(.*\) // import.*;package \1;' $(git grep -l '^package.*// import' | grep -v 'vendor/')
Everything was included, except for
package labels // import k8s.io/kubernetes/pkg/util/labels
because that package is marked as "read-only".
Previously, ValidateNodeSelector did not check that labels are valid. Now it
does for resource.k8s.io, regardless whether an object already was created with
invalid labels in an earlier Kubernetes release. Theoretically this is a
breaking change and could cause problems during an upgrade, but that is highly
unlikely in practice.
In contrast to node affinity, DRA does not ignore parse errors
(= uses NewNodeSelector, not NewLazyErrorNodeSelector), so invalid labels would
have been found instead of being silently ignored.
Even if some object has invalid labels, this only affects an alpha -> beta
upgrade which isn't guaranteed to work seamlessly.
