-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
tests: demonstrate issue with JSON patch when base adds name prefix #971
tests: demonstrate issue with JSON patch when base adds name prefix #971
Conversation
version: v1 | ||
kind: Service | ||
name: myService # BUG: this should be a-myService, because that is what the output for the base contains | ||
path: service-patch.yaml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if I fully understand, the test fails if name
is myService, and you expect name
to be set to a-myService?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test passes as it is written now. But I consider that a bug (see #972) and would expect the test to pass with name: a-myService
instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great. Can you add a reference to 972 in the comment, or better yet in the name of the test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
But feel free to add that 972 reference, and i'll re-lgtm it.
Also please update the issue to point to this test once it's merged. :)
The expectation is that the base entity can be referenced by its name with prefix, because the overlay shouldn't have to know how the base is generated. But currently the entity is only found when using the name without prefix. Related-To: kubernetes-sigs#972
dc3376f
to
403ede7
Compare
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: pohly If they are not already assigned, you can assign the PR to them by writing The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Jeff Regan <notifications@github.com> writes:
Can you add a reference to 972 in the comment, or better yet in the
name of the test?
Chicken-and-egg problem ;-) First came the test and PR, then the issue report.
I've updated the code and the commit message, both link to the issue now.
|
yay, thanks /lgtm |
The expectation is that the base entity can be referenced by its name
with prefix, because the overlay shouldn't have to know how the base
is generated. But currently the entity is only found when using the
name without prefix.