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.

Sign up for GitHub

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

Jump to bottom

Reconcile examples dir with config/samples #20

Closed
bobcatfish opened this issue Sep 8, 2018 · 2 comments
Closed

Reconcile examples dir with config/samples #20

bobcatfish opened this issue Sep 8, 2018 · 2 comments
Assignees
@bobcatfish

Comments

@bobcatfish
Copy link
Collaborator

Expected Behavior

We should have one clear location for all of our example yamls and it should be tested (and eventually included in our CI - #15).

Actual Behavior

We currently have two sets of examples:

  1. config/samples - these were generated by kubebuilder and we modified them
  2. examples these were created specifically to demonstrate the propose pipeline API to the build working group

config/samples is used in our unit tests but examples is not.

Additional Info

There is some overlap with #3 and @tejal29 will need to be consulted re. the plan around those samples so that:

  1. All of our samples live in one place
  2. All of our samples are tested to ensure they continue working
@bobcatfish bobcatfish mentioned this issue Sep 19, 2018
Merged
@bobcatfish bobcatfish self-assigned this Sep 19, 2018
@bobcatfish bobcatfish added this to the Mid October Demo milestone Sep 21, 2018
@bobcatfish
Copy link
Collaborator Author

I think we probably want to keep examples but I just noticed that the config/samples examples don't include any Pipeline level templating.

@bobcatfish
Copy link
Collaborator Author

Well neither does config/samples so ¯_(ツ)_/¯

@bobcatfish bobcatfish mentioned this issue Sep 27, 2018
Merged
@bobcatfish bobcatfish removed this from the Mid October Demo milestone Oct 3, 2018
@bobcatfish bobcatfish mentioned this issue Oct 8, 2018
Closed
bobcatfish referenced this issue in bobcatfish/pipeline Oct 8, 2018
@bobcatfish
Only have one set of examples and test that they are valid
2b31251 
We ended up in a state where we had both an `examples` dir and a
`samples` dir, and neither were tested, so both were slightly out of
sync with each other and the actual types.

This was b/c:
- When we used kubebuilder, we generated our types from an original set
  of examples - previously these were in `config/samples` and they ended
  up in `samples`. We used the validation tests generated by kubebuilder
  to test these, but when we removed kubebuilder we removed the tests
- Before presenting the API to the Build CRD working group, we wanted to
  have some more complex, real world examples, which @tejal29 created
  from the `kritis` project and the k8s guestbook example

This commit makes sure that all of the functionality demonstrated in
`samples` is in `examples`, and removes `samples`. This included:
- Fixing `passedConstraints` to be plural as the types expected
- Including references to the `PipelineParams` in the `TaskRun` example
- Fixing clusterBindings to refer to the actual names of the clusters
  involved

It also adds a step to the integration tests to deploy the examples,
which will fail if they do not match the expected schema. At this point
they are torn down immediately after creation, but in #108 we can expand
this to actually test that they are working.

Also had to make some tweaks to the types:
- actually including `ClusterBindings` in the `Pipeline`
- Using names which include `Resource` instead of `Source`, which we
  moved away from in #39

Fixes #20
bobcatfish referenced this issue in bobcatfish/pipeline Oct 8, 2018
@bobcatfish
Only have one set of examples and test that they are valid
62cf6c3 
We ended up in a state where we had both an `examples` dir and a
`samples` dir, and neither were tested, so both were slightly out of
sync with each other and the actual types.

This was b/c:
- When we used kubebuilder, we generated our types from an original set
  of examples - previously these were in `config/samples` and they ended
  up in `samples`. We used the validation tests generated by kubebuilder
  to test these, but when we removed kubebuilder we removed the tests
- Before presenting the API to the Build CRD working group, we wanted to
  have some more complex, real world examples, which @tejal29 created
  from the `kritis` project and the k8s guestbook example

This commit makes sure that all of the functionality demonstrated in
`samples` is in `examples`, and removes `samples`. This included:
- Fixing `passedConstraints` to be plural as the types expected
- Including references to the `PipelineParams` in the `TaskRun` example
  (removing the duplicated list of result endpoints, since we'll be
  getting this from the references instead)
- Fixing clusterBindings to refer to the actual names of the clusters
  involved

It also adds a step to the integration tests to deploy the examples,
which will fail if they do not match the expected schema. At this point
they are torn down immediately after creation, but in #108 we can expand
this to actually test that they are working.

Also had to make some tweaks to the types:
- actually including `ClusterBindings` in the `Pipeline`
- Using names which include `Resource` instead of `Source`, which we
  moved away from in #39

Fixes #20
@bobcatfish bobcatfish mentioned this issue Oct 8, 2018
Merged
bobcatfish referenced this issue in bobcatfish/pipeline Oct 8, 2018
@bobcatfish
Only have one set of examples and test that they are valid
affcf9a 
We ended up in a state where we had both an `examples` dir and a
`samples` dir, and neither were tested, so both were slightly out of
sync with each other and the actual types.

This was b/c:
- When we used kubebuilder, we generated our types from an original set
  of examples - previously these were in `config/samples` and they ended
  up in `samples`. We used the validation tests generated by kubebuilder
  to test these, but when we removed kubebuilder we removed the tests
- Before presenting the API to the Build CRD working group, we wanted to
  have some more complex, real world examples, which @tejal29 created
  from the `kritis` project and the k8s guestbook example

This commit makes sure that all of the functionality demonstrated in
`samples` is in `examples`, and removes `samples`. This included:
- Fixing `passedConstraints` to be plural as the types expected
- Including references to the `PipelineParams` in the `TaskRun` example
  (removing the duplicated list of result endpoints, since we'll be
  getting this from the references instead)
- Fixing clusterBindings to refer to the actual names of the clusters
  involved

It also adds a step to the integration tests to deploy the examples,
which will fail if they do not match the expected schema. At this point
they are torn down immediately after creation, but in #108 we can expand
this to actually test that they are working.

Also had to make some tweaks to the types:
- actually including `ClusterBindings` in the `Pipeline`
- Using names which include `Resource` instead of `Source`, which we
  moved away from in #39

Fixes #20
bobcatfish referenced this issue in bobcatfish/pipeline Oct 9, 2018
@bobcatfish
Only have one set of examples and test that they are valid
91793b5 
We ended up in a state where we had both an `examples` dir and a
`samples` dir, and neither were tested, so both were slightly out of
sync with each other and the actual types.

This was b/c:
- When we used kubebuilder, we generated our types from an original set
  of examples - previously these were in `config/samples` and they ended
  up in `samples`. We used the validation tests generated by kubebuilder
  to test these, but when we removed kubebuilder we removed the tests
- Before presenting the API to the Build CRD working group, we wanted to
  have some more complex, real world examples, which @tejal29 created
  from the `kritis` project and the k8s guestbook example

This commit makes sure that all of the functionality demonstrated in
`samples` is in `examples`, and removes `samples`. This included:
- Fixing `passedConstraints` to be plural as the types expected
- Including references to the `PipelineParams` in the `TaskRun` example
  (removing the duplicated list of result endpoints, since we'll be
  getting this from the references instead)
- Fixing clusterBindings to refer to the actual names of the clusters
  involved

It also adds a step to the integration tests to deploy the examples,
which will fail if they do not match the expected schema. At this point
they are torn down immediately after creation, but in #108 we can expand
this to actually test that they are working.

Also had to make some tweaks to the types:
- actually including `ClusterBindings` in the `Pipeline`
- Using names which include `Resource` instead of `Source`, which we
  moved away from in #39

Fixes #20
knative-prow-robot pushed a commit that referenced this issue Oct 9, 2018
@bobcatfish @knative-prow-robot
Only have one set of examples and test that they are valid
9031ed4 
We ended up in a state where we had both an `examples` dir and a
`samples` dir, and neither were tested, so both were slightly out of
sync with each other and the actual types.

This was b/c:
- When we used kubebuilder, we generated our types from an original set
  of examples - previously these were in `config/samples` and they ended
  up in `samples`. We used the validation tests generated by kubebuilder
  to test these, but when we removed kubebuilder we removed the tests
- Before presenting the API to the Build CRD working group, we wanted to
  have some more complex, real world examples, which @tejal29 created
  from the `kritis` project and the k8s guestbook example

This commit makes sure that all of the functionality demonstrated in
`samples` is in `examples`, and removes `samples`. This included:
- Fixing `passedConstraints` to be plural as the types expected
- Including references to the `PipelineParams` in the `TaskRun` example
  (removing the duplicated list of result endpoints, since we'll be
  getting this from the references instead)
- Fixing clusterBindings to refer to the actual names of the clusters
  involved

It also adds a step to the integration tests to deploy the examples,
which will fail if they do not match the expected schema. At this point
they are torn down immediately after creation, but in #108 we can expand
this to actually test that they are working.

Also had to make some tweaks to the types:
- actually including `ClusterBindings` in the `Pipeline`
- Using names which include `Resource` instead of `Source`, which we
  moved away from in #39

Fixes #20
vdemeester referenced this issue in openshift/tektoncd-pipeline Apr 5, 2019
@bobcatfish @knative-prow-robot
Only have one set of examples and test that they are valid
71b7abc 
We ended up in a state where we had both an `examples` dir and a
`samples` dir, and neither were tested, so both were slightly out of
sync with each other and the actual types.

This was b/c:
- When we used kubebuilder, we generated our types from an original set
  of examples - previously these were in `config/samples` and they ended
  up in `samples`. We used the validation tests generated by kubebuilder
  to test these, but when we removed kubebuilder we removed the tests
- Before presenting the API to the Build CRD working group, we wanted to
  have some more complex, real world examples, which @tejal29 created
  from the `kritis` project and the k8s guestbook example

This commit makes sure that all of the functionality demonstrated in
`samples` is in `examples`, and removes `samples`. This included:
- Fixing `passedConstraints` to be plural as the types expected
- Including references to the `PipelineParams` in the `TaskRun` example
  (removing the duplicated list of result endpoints, since we'll be
  getting this from the references instead)
- Fixing clusterBindings to refer to the actual names of the clusters
  involved

It also adds a step to the integration tests to deploy the examples,
which will fail if they do not match the expected schema. At this point
they are torn down immediately after creation, but in #108 we can expand
this to actually test that they are working.

Also had to make some tweaks to the types:
- actually including `ClusterBindings` in the `Pipeline`
- Using names which include `Resource` instead of `Source`, which we
  moved away from in #39

Fixes #20
chmouel pushed a commit to chmouel/tektoncd-pipeline that referenced this issue Apr 29, 2019
@vdemeester
Merge pull request tektoncd#20 from chmouel/mergemaster
4585a04 
Merge change to master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bobcatfish bobcatfish

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@bobcatfish