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

Concise Yaml for pipeline declaration. #138

Closed
tejal29 opened this issue Oct 12, 2018 · 9 comments · Fixed by #292
Closed

Concise Yaml for pipeline declaration. #138

tejal29 opened this issue Oct 12, 2018 · 9 comments · Fixed by #292

Comments

@tejal29
Copy link
Contributor

tejal29 commented Oct 12, 2018

Expected Behavior

user should be able to write minimal lines of yaml to declare a pipeline.

Actual Behavior

we end up writing a number of redundant fields like "name" in inputSourceBindings might not be required.

Steps to Reproduce the Problem

  1. Try writing a sample pipeline
  2. Remove repeated fields.

Additional Info

@tejal29
Copy link
Contributor Author

tejal29 commented Oct 18, 2018

One way to address this is removing TaskRef and using a string instead.

@tejal29 tejal29 self-assigned this Oct 18, 2018
bobcatfish added a commit to bobcatfish/pipeline that referenced this issue Oct 25, 2018
Before this change, Tasks were retrieving the Resources to use by
looking for PipelineResources with exactly the name that the Resource is
declared with in the Task. This means that the Resource binding in
Pipelines (and TaskRuns) was doing absolutely nothing.

Instead, we now map bound Resources inside of TaskRuns. The name of a
Resource declared in a Task is a _parameter_, which we fulfill by
providing the _key_ aka parameter of the Task, and the corresponding
Resource reference we should use for that _key_.

This allows us to use the same Task with different Resources without
having to change the Task or the Resource.

This was not caught by any tests because the tests were co-incidentally
using the same name for the Resources and the Resource
key/name/parameter in the Task. Now the tests use different names.

This came up while creating example yaml for #89 - as soon as I tried to
use two different git resources (one upstream, one my fork) I realized
that the mapping was not working.

This also partially addresses tektoncd#138 by removing the `Name` field from the
bindings, which is not required.
@bobcatfish
Copy link
Collaborator

I took a quick look through our example yaml and these are the things I noticed that we might not need:

At the moment it also looks like params could be simpler - since they are currently just a name - but I think instead of simplifying this we should actually add more fields, see #190.

bobcatfish added a commit to bobcatfish/pipeline that referenced this issue Oct 25, 2018
Before this change, Tasks were retrieving the Resources to use by
looking for PipelineResources with exactly the name that the Resource is
declared with in the Task. This means that the Resource binding in
Pipelines (and TaskRuns) was doing absolutely nothing.

Instead, we now map bound Resources inside of TaskRuns. The name of a
Resource declared in a Task is a _parameter_, which we fulfill by
providing the _key_ aka parameter of the Task, and the corresponding
Resource reference we should use for that _key_.

This allows us to use the same Task with different Resources without
having to change the Task or the Resource.

This was not caught by any tests because the tests were co-incidentally
using the same name for the Resources and the Resource
key/name/parameter in the Task. Now the tests use different names.

This came up while creating example yaml for #89 - as soon as I tried to
use two different git resources (one upstream, one my fork) I realized
that the mapping was not working.

This also partially addresses tektoncd#138 by removing the `Name` field from the
bindings, which is not required.
bobcatfish added a commit to bobcatfish/pipeline that referenced this issue Oct 25, 2018
Before this change, Tasks were retrieving the Resources to use by
looking for PipelineResources with exactly the name that the Resource is
declared with in the Task. This means that the Resource binding in
Pipelines (and TaskRuns) was doing absolutely nothing.

Instead, we now map bound Resources inside of TaskRuns. The name of a
Resource declared in a Task is a _parameter_, which we fulfill by
providing the _key_ aka parameter of the Task, and the corresponding
Resource reference we should use for that _key_.

This allows us to use the same Task with different Resources without
having to change the Task or the Resource.

This was not caught by any tests because the tests were co-incidentally
using the same name for the Resources and the Resource
key/name/parameter in the Task. Now the tests use different names.

This came up while creating example yaml for #89 - as soon as I tried to
use two different git resources (one upstream, one my fork) I realized
that the mapping was not working.

This also partially addresses tektoncd#138 by removing the `Name` field from the
bindings, which is not required.

This addresses part of #64 by adding templating from outputs.
@abayer
Copy link
Contributor

abayer commented Oct 26, 2018

At @dlorenc's talk at Devops World/Jenkins World (augh awkward long conference names), and in discussions he and I had during the conference, he advocated for the idea that the Build Pipeline YAML is the "machine code" (for lack of a better term), which can be generated either via an editor or through transformation from another syntax. I'm intending down the road to add support for transforming Jenkins Declarative Pipeline syntax into Build Pipeline YAML, e.g.

I mention this because I think it is important to have the canonical representation be clearly unambiguous and useful for transformation from/to other forms, and for utilization in whatever implementation of Build Pipeline. That said, the particular cases cited above do seem to not create any ambiguity if they were removed, so hey. =)

@bobcatfish
Copy link
Collaborator

he advocated for the idea that the Build Pipeline YAML is the "machine code" (for lack of a better term), which can be generated either via an editor or through transformation from another syntax

That's an interesting point @abayer , so I think this would imply that the users of the YAML are the tools generating it, and the APIs consuming it, but not actually the end users of the Pipeline itself, is that accurate?

(+ @tejal29 )

@abayer
Copy link
Contributor

abayer commented Oct 29, 2018

Well, optionally - it may be better to think of it as end users may be writing the YAML directly, but an implementation/utilization of Build Pipeline may use a different syntax (like I mentioned, in my specific case, I'm thinking about migration paths for existing Jenkins users of its Declarative Pipeline syntax) that "transpiles" down to the standard YAML.

My thinking is heavily influenced by the idea of Build Pipeline as a spec as much as an implementation - I'm focused on the authoring experience (admittedly from the Jenkins perspective currently) and absolutely adore the idea of a standard definition of what a Pipeline can do that you can expect to have working on any implementation of the Build Pipeline "spec", with at the very least the same conceptual model and building blocks across the board.

@bobcatfish bobcatfish assigned bobcatfish and unassigned tejal29 Oct 29, 2018
@bobcatfish
Copy link
Collaborator

bobcatfish commented Oct 29, 2018

Note to self: Also need to update user study in @tejal29 's branch and decide where to put user study.

@tejal29
Copy link
Contributor Author

tejal29 commented Oct 30, 2018

@abayer i agree with your view that the yaml should be as unambiguous as it can and if it leads to very very long config, we can create tools for better user experience.
However, my initial concern is to make sure we don't repel users because of these long configs.
We need to make sure, we are not adding to the user pain.

@tejal29
Copy link
Contributor Author

tejal29 commented Oct 30, 2018

he advocated for the idea that the Build Pipeline YAML is the "machine code" (for lack of a better term), which can be generated either via an editor or through transformation from another syntax

That's an interesting point @abayer , so I think this would imply that the users of the YAML are the tools generating it, and the APIs consuming it, but not actually the end users of the Pipeline itself, is that accurate?

(+ @tejal29 )

yes, but we need to support both consumers (real users and tool) until we provide a tool which does that.

@dlorenc
Copy link
Contributor

dlorenc commented Oct 30, 2018

To clarify my thoughts a little further - we'll basically have two classes of users here (I think this is true for basically all Kubernetes yamls today): the users interfacing with Kubernetes directly and the users interfacing with higher level tooling built on top of Kubernetes.

In the long term we'll likely have way more of the second class of user - people using this through other systems. In the short term, we don't have any of those systems yet :)

I think we need to make sure the system is designed for both use-cases. YAML isn't a great user experience no matter how simple we make it, so we should expect and design for systems built on top. But that's not an excuse to make this overly verbose and complicated now, otherwise no one will be able to understand it enough to build something on top.

bobcatfish added a commit to bobcatfish/pipeline that referenced this issue Oct 30, 2018
Before this change, Tasks were retrieving the Resources to use by
looking for PipelineResources with exactly the name that the Resource is
declared with in the Task. This means that the Resource binding in
Pipelines (and TaskRuns) was doing absolutely nothing.

Instead, we now map bound Resources inside of TaskRuns. The name of a
Resource declared in a Task is a _parameter_, which we fulfill by
providing the _key_ aka parameter of the Task, and the corresponding
Resource reference we should use for that _key_.

This allows us to use the same Task with different Resources without
having to change the Task or the Resource.

This was not caught by any tests because the tests were co-incidentally
using the same name for the Resources and the Resource
key/name/parameter in the Task. Now the tests use different names.

This came up while creating example yaml for #89 - as soon as I tried to
use two different git resources (one upstream, one my fork) I realized
that the mapping was not working.

This also partially addresses tektoncd#138 by removing the `Name` field from the
bindings, which is not required.

This addresses part of #64 by adding templating from outputs.
knative-prow-robot pushed a commit that referenced this issue Oct 30, 2018
Before this change, Tasks were retrieving the Resources to use by
looking for PipelineResources with exactly the name that the Resource is
declared with in the Task. This means that the Resource binding in
Pipelines (and TaskRuns) was doing absolutely nothing.

Instead, we now map bound Resources inside of TaskRuns. The name of a
Resource declared in a Task is a _parameter_, which we fulfill by
providing the _key_ aka parameter of the Task, and the corresponding
Resource reference we should use for that _key_.

This allows us to use the same Task with different Resources without
having to change the Task or the Resource.

This was not caught by any tests because the tests were co-incidentally
using the same name for the Resources and the Resource
key/name/parameter in the Task. Now the tests use different names.

This came up while creating example yaml for #89 - as soon as I tried to
use two different git resources (one upstream, one my fork) I realized
that the mapping was not working.

This also partially addresses #138 by removing the `Name` field from the
bindings, which is not required.

This addresses part of #64 by adding templating from outputs.
@bobcatfish bobcatfish added this to the 0.0.1 Alpha release milestone Nov 3, 2018
bobcatfish added a commit to bobcatfish/pipeline that referenced this issue Dec 1, 2018
In the interests in keeping our spec as lean and simple as it can
possibly be, at the moment there is no reason why our Results config
needs to have a name, so let's remove it.

This is the last unnecessary field we have in our yamls that I'm aware
of, so this fixes tektoncd#138
bobcatfish added a commit to bobcatfish/pipeline that referenced this issue Dec 1, 2018
In the interests in keeping our spec as lean and simple as it can
possibly be, at the moment there is no reason why our Results config
needs to have a name, so let's remove it.

This is the last unnecessary field we have in our yamls that I'm aware
of, so this fixes tektoncd#138
bobcatfish added a commit to bobcatfish/pipeline that referenced this issue Dec 1, 2018
In the interests in keeping our spec as lean and simple as it can
possibly be, at the moment there is no reason why our Results config
needs to have a name, so let's remove it.

This is the last unnecessary field we have in our yamls that I'm aware
of, so this fixes tektoncd#138
bobcatfish added a commit to bobcatfish/pipeline that referenced this issue Dec 1, 2018
In the interests in keeping our spec as lean and simple as it can
possibly be, at the moment there is no reason why our Results config
needs to have a name, so let's remove it.

This is the last unnecessary field we have in our yamls that I'm aware
of, so this fixes tektoncd#138
knative-prow-robot pushed a commit that referenced this issue Dec 4, 2018
In the interests in keeping our spec as lean and simple as it can
possibly be, at the moment there is no reason why our Results config
needs to have a name, so let's remove it.

This is the last unnecessary field we have in our yamls that I'm aware
of, so this fixes #138
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants