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

Make Tasks/TaskRuns PodSpecable #2327

Closed
mattmoor opened this issue Apr 4, 2020 · 7 comments
Closed

Make Tasks/TaskRuns PodSpecable #2327

mattmoor opened this issue Apr 4, 2020 · 7 comments
Labels
kind/design Categorizes issue or PR as related to design. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.

Comments

@mattmoor
Copy link
Member

mattmoor commented Apr 4, 2020

This is bound to be contentious because it's a disruptive change, but it's more tolerable with the emergence of Conversion webhooks. 🎉

In Knative, we have been exploring the idea of Bindings as a way to project information into "Pod Specable" things. We in fact adjusted our own Serving resource shapes to align with this shape in our own path to "v1".

The basic idea is that if the core resource adheres to the following shape, then we can apply a common class of applications to it.

spec:
  template:  # corev1.PodTemplateSpec
    spec:  # corev1.PodSpec
      containers:
      - ...  # corev1.Container

Today, I can apply these bindings to Jobs, which I have found immensely useful, but it pains me that I can't use these with Tekton Task[Run]s. 😭

One of the really nice things about Bindings is that they decouple things in ways that allow you to ship more reusable applications (in this case Tasks) that are bound when they are installed into a particular context (e.g. cluster / namespace).

I'd be happy to discuss the benefits / pitfalls of this, but it's an area that I feel has a ton of potential, and so I wanted to seed a convo here.

@imjasonh
Copy link
Member

imjasonh commented Apr 5, 2020

Can you elaborate how you imagine this looking in a Task spec? And what using this would unlock for Tekton users/integrators?

One possible wrinkle is that we've actually added fields beyond what the Container type supports (script:) and will likely find reasons to do it again in the future, possibly also at the PodSpec level. Is the binding code designed to support this case as well?

Also, there are PodSpec and Container fields we don't intend to support and would like to disallow (e.g., we don't let steps specify terminationMessagePath, and health checks are probably meaningless). Can bindings support this use case generally?

@mattmoor
Copy link
Member Author

mattmoor commented Apr 5, 2020

Extensions to the spec should be fine since everything is done via PATCH. Restrictions to the spec are also generally not a problem as validation will reject invalid mutations (and some bindings simply won't work).

If you look at the K8s app resources this second part is sort of true there as well, but less in terms of field validation and more in terms of enum arity. Take for example restartPolicy; Job requires Never (maybe it allows OnFailure too?), but Deployment (and others) require Always.

The most obvious example of value this provides is environmental auth bindings, which can handle augmenting the runtime environment with authentication information. To pick a few examples just in this domain:

  1. Mounting in service account secret keys and setting Google Application Default credentials up (useful for running the same Googley things off GCE, e.g. on-prem).
  2. Mounting in distinct K8s auth/endpoint info to manipulate remote clusters (this is something I've been thinking about from the perspective of running cross-cluster controllers).
  3. DOCKER_CONFIG handling could be reformulated as a type of Binding.
  4. Git credential handling could be reformulated as a type of Binding.

I think trying to formulate 3. and 4. via an extension mechanism like Bindings actually sets you up better to support things unlikely to land in-tree (e.g. hg support, auth for publishing rando artifacts like Rust crates or conan.io C++ modules). To make an analogy, eventually Jenkins shifted all of its core functionality to work through the plugin APIs.

  1. Binding proprietary auth is a big one. I have a GithubBinding I use with ksvc (for use with the github API client), but I have to manually implement that same contract in the Tekton tasks I run to generate PRs. I have a similar VMware-specific case, but I'm trying to keep my vendor hat off 😇

I could also see this being used to connect tasks to auditing systems when used in production contexts, but not when running in development contexts[1]. I'd like to see this shape how we handle Observability in general, which is typically a highly context-sensitive endeavor.

If I had to tl;dr the goal of Bindings, it lets us separate (possibly context-sensitive) business logic from the context, and let Bindings re-infuse that context based on the environment it runs in.


[1] - This is a corollary to the canonical service-side example of connecting to the prod database in prod, and non-prod elsewhere.

@ghost ghost added the kind/design Categorizes issue or PR as related to design. label Apr 6, 2020
@ghost
Copy link

ghost commented Apr 24, 2020

Just to note: we've actively discussed getting away from containers / PodSpec / kubernetes-isms in several issues.

So maybe this could be something we do but we'd also like the option to not be tied to the Pod Spec (I think?).

@mattmoor
Copy link
Member Author

I don't want to launch into an "extensibility" debate here, but moving away from PodSpec-like things because you want to support non-Pod-based tasks feels like you are sorely missing an extensibility story (which is IMO a key ingredient of the incumbent's meteoric success). Please embrace extensibility over "kitchen sink" resources 🙏

@ghost
Copy link

ghost commented Apr 24, 2020

My reading of this issue is that it's squarely aimed at changing Tekton's default Task shape to adhere more closely to PodSpec. We've specifically pushed back against this in the past and it's worth keeping that context in mind when a change like this is being discussed. It's a mis-characterization to suggest that doing so argues one way or the other on the question of extensibility.

It's possible for both of these things to be true: Tekton's Task shape should not be constrained or tied to PodSpec AND we should introduce extensibility such that users who want a type that adheres closely to PodSpec are well-catered for.

@tekton-robot
Copy link
Collaborator

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close.

/lifecycle stale

Send feedback to tektoncd/plumbing.

@tekton-robot
Copy link
Collaborator

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
If this issue is safe to close now please do so with /close.

/lifecycle rotten

Send feedback to tektoncd/plumbing.

@tekton-robot tekton-robot added lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Aug 14, 2020
@mattmoor mattmoor closed this as completed Nov 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/design Categorizes issue or PR as related to design. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.
Projects
None yet
Development

No branches or pull requests

3 participants