Remove "template.openshift.io/template-instance" label#16808
Conversation
openshift-ci-robot
added
the
size/XL
| kapi.Kind("Secret"), | ||
| kapi.Kind("Service"), | ||
| routeapi.Kind("Route"), | ||
| routeapi.LegacyKind("Route"): |
There was a problem hiding this comment.
does this mean the expose annotation can only be used on these object types?
There was a problem hiding this comment.
Yes; that is currently the case anyway.
There was a problem hiding this comment.
Nothing in the code should prevent expansion in the future.
There was a problem hiding this comment.
hm, i guess that was a choice we made, sorry, i forgot about it.
There was a problem hiding this comment.
Why is legacy only an issue for Route? aren't there also Legacy apis for the other types?
There was a problem hiding this comment.
Legacy is via /oapi, non-legacy is via /apis/. Core Kubernetes API objects haven't moved from /api into /apis/, at least at this time.
There was a problem hiding this comment.
hm. I thought it was also group vs non-group apis?
There was a problem hiding this comment.
i guess you're saying the k8s objects haven't been groupified. ok.
| o.Expect(err).NotTo(o.HaveOccurred()) | ||
|
|
||
| err = cli.AsAdmin().Run("create").Args("-f", "-").InputString(stdout).Execute() | ||
| o.Expect(err).NotTo(o.HaveOccurred()) |
There was a problem hiding this comment.
Previously there was a matching "delete", but I took it out. Will change.
|
|
||
| // TemplateInstanceLabel is used to label every object created by the | ||
| // TemplateInstance API. | ||
| TemplateInstanceLabel = "template.openshift.io/template-instance" |
There was a problem hiding this comment.
@spadgett fyi. don't know if you were using this, but we're taking it away in preference to using ownerRefs to track the templateinstance associated with an object.
There was a problem hiding this comment.
Thanks for the heads up. We're not using it.
jim-minter
force-pushed
the
remove_templateinstance_label
branch
from
83d9acf to
e227d13
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bparees, jim-minter The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
openshift-merge-robot
added
the
approved
|
flake #16836 |
|
@jim-minter: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
Automatic merge from submit-queue (batch tested with PRs 16777, 16811, 16823, 16808, 16833). |
|
/test all [submit-queue is verifying that this PR is safe to merge] |
6 participants
Background: this label was a stopgap before OwnerReferences and TemplateInstance.Status.Objects existed. Now that these do, and before the templateinstance API is officially released, I'd like to remove the label. It is unnecessary, and it is problematic on two fronts: it is effectively trying to be a ObjectReference, but it doesn't record Namespace or UID. That means that it it is broken in the cases of cross-namespace template instantiations (TemplateInstance in one namespace, instantiated object in another), and rapid creation/deletion of TemplateInstances (recently witnessed in an extended test flake).