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

Proposal: Tekton objects should be self descriptive using annotations instead of insider info (container name prefix...) #961

Closed
joseblas opened this issue Jun 6, 2019 · 6 comments
Labels
design This task is about creating and discussing a design help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature.

Comments

@joseblas
Copy link
Contributor

joseblas commented Jun 6, 2019

Expected Behavior

Tekton Step names shouldn't have any constrains but length.

Actual Behavior

Currently Tekton sidecars container names is based on steps names, and this makes Tekton step names not flexible.

Proposal

When a Pod is created should have an annotation with all the Tekton containers within. Something like Istio does:
sidecar.istio.io/status: '{"version":"299b0fe3441985f893a8b9fcca72a53989f22ef3d7b53148499683a078e54915","initContainers":["istio-init"],"containers":["istio-proxy"],"volumes":["istio-envoy","istio-certs"],"imagePullSecrets":null}'

@vdemeester
Copy link
Member

This is related to that change and #936. We should try to make sure our object are as self descriptive as they can be and use annotation more instead of having to rely on insider information (aka the containername prefix, …)

@vdemeester vdemeester added design This task is about creating and discussing a design help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature. labels Jun 7, 2019
@vtereso
Copy link

vtereso commented Jun 7, 2019

In terms of processing, it requires less memory to filter the steps out of the pod using a prefix rather than having to compare against all annotations in a map (although either is fine). However, one question is why all containers within the task pod are not steps? A task in this manner would be most descriptive imo. EDIT: I suppose this is not possible because sidecars?

@joseblas
Copy link
Contributor Author

joseblas commented Jun 7, 2019

For instance, Istio adds sidecars containers in pods; when Tekton containers are finished the pod is finished unless a sidecar container is still running. That container has to be killed. Up to now, the code was based on prefix on step names. The goal of this thread is to provide an alternative design.

@joseblas joseblas changed the title Make sure Tekton can find out if there is any sidecar running Tekton objects are self descriptive using annotations instead of insider info (container name prefix...) Jun 11, 2019
@joseblas
Copy link
Contributor Author

Hi @vdemeester @bobcatfish,
this was open last week and no feedback since. I guess this proposal could be used in our implementation and close this issue, or discuss it in our weekly meeting

@vdemeester vdemeester changed the title Tekton objects are self descriptive using annotations instead of insider info (container name prefix...) Proposal: Tekton objects should be self descriptive using annotations instead of insider info (container name prefix...) Jun 12, 2019
@joseblas
Copy link
Contributor Author

implemented in #936

@bobcatfish
Copy link
Collaborator

Sorry for the lack of discussion in the issue @joseblas ! I had a pretty big backlog of issues to get through and didn't get a chance to get back to this one :(

In our working group meeting last week we discussed #1027 which seems like it might accomplish what you are looking for by making it so that container names are included in statuses as well as step names? (It isn't using annotations like you described here tho)

Please re-open this issue if #1027 doesn't address what you wanted - I took a look at #936 and I'm not 100% sure it addresses this issue, but I might be missing something!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design This task is about creating and discussing a design help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

No branches or pull requests

4 participants