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

cleanup to resource types #73

Merged
merged 5 commits into from
Oct 1, 2018
Merged

cleanup to resource types #73

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
213e26f
cleanup to resource types
nader-ziada Sep 27, 2018
81b7b71
update example yaml with better resource names
nader-ziada Sep 27, 2018
2145e1e
updates to resources design
nader-ziada Sep 28, 2018
99d5fb6
add type field to resource in task
nader-ziada Sep 29, 2018
d4059d6
Merge branch 'master' into resource-followup
nader-ziada Oct 1, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 18 additions & 20 deletions config/300-pipeline.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,25 +22,6 @@ spec:
type: object
spec:
properties:
resources:
items:
properties:
name:
type: string
resourceRef:
properties:
apiVersion:
type: string
name:
type: string
required:
- name
type: object
required:
- name
- resourceRef
type: object
type: array
tasks:
items:
properties:
Expand All @@ -63,6 +44,15 @@ spec:
type: string
name:
type: string
resourceRef:
properties:
apiVersion:
type: string
name:
type: string
required:
- name
type: object
passedConstraints:
items:
type: string
Expand All @@ -81,6 +71,15 @@ spec:
type: string
name:
type: string
resourceRef:
properties:
apiVersion:
type: string
name:
type: string
required:
- name
type: object
passedConstraints:
items:
type: string
Expand Down Expand Up @@ -118,7 +117,6 @@ spec:
type: array
required:
- tasks
- resources
type: object
status:
type: object
Expand Down
26 changes: 9 additions & 17 deletions config/300-resource.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,33 +22,25 @@ spec:
type: object
spec:
properties:
resources:
name:
type: string
params:
items:
properties:
name:
type: string
params:
items:
properties:
name:
type: string
value:
type: string
required:
- name
- value
type: object
type: array
type:
value:
type: string
required:
- name
- type
- params
- value
type: object
type: array
type:
type: string
required:
- resources
- type
- params
type: object
status:
type: object
Expand Down
23 changes: 3 additions & 20 deletions config/300-task.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,18 +86,11 @@ spec:
properties:
name:
type: string
resourceRef:
properties:
apiVersion:
type: string
name:
type: string
required:
- name
type: object
type:
type: string
required:
- name
- resourceRef
- type
type: object
type: array
type: object
Expand All @@ -108,18 +101,8 @@ spec:
properties:
name:
type: string
resourceRef:
properties:
apiVersion:
type: string
name:
type: string
required:
- name
type: object
required:
- name
- resourceRef
type: object
type: array
results:
Expand Down
12 changes: 5 additions & 7 deletions examples/build_task.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,22 +1,20 @@
apiVersion: pipeline.knative.dev/v1beta1
apiVersion: pipeline.knative.dev/v1alpha1
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whoops, can't believe how easy it is to miss these! Thanks for catching it :D

kind: Task
metadata:
name: build-push
namespace: default
spec:
inputs:
resources:
- resourceRef:
name: resource-name
name: workspace
- name: workspace
type: git
params:
- name: pathToDockerFile
value: string
outputs:
resources:
- resourceRef:
name: registry
name: builtImage
- name: builtImage
type: image
buildSpec:
template:
name: kaniko
Expand Down
10 changes: 4 additions & 6 deletions examples/deploy_tasks.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@ metadata:
spec:
inputs:
resources:
- resourceRef:
name: resource-name
name: workspace
- name: workspace
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think this should have a type as well, in this case git or github, so that a person using the Task knows what kind of resource to provide

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will add type, makes sense for clarity

type: git
params:
- name: pathToHelmCharts
value: string
Expand All @@ -33,9 +32,8 @@ metadata:
spec:
inputs:
resources:
- resourceRef:
name: resource-name
name: workspace
- name: workspace
type: git
params:
- name: kubectlArgs
value: string
Expand Down
65 changes: 43 additions & 22 deletions examples/pipelines/guestbook-resources.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,30 +1,51 @@
apiVersion: pipeline.knative.dev/v1alpha1
kind: Resource
metadata:
name: guestbook-resources-sample
name: guestbook-resources-git
namespace: default
spec:
resources:
- name: guestbook
type: git
params:
type: git
params:
- name: url
value: github.com/kubernetes/examples
- name: revision
value: HEAD
- name: serviceAccount
value: githubServiceAccount
---
apiVersion: pipeline.knative.dev/v1alpha1
kind: Resource
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interesting! so one would need to reference each Resource separately?

I am fine with doing this but I assumed differently in the examples I wrote out, e.g. in #64 I assumed you could declare one Resource type which contained a list of all the resources you needed for your Pipeline. One thing I like about that is that it means the number of yaml files a person would need to interact with their Pipeline is limited.

But if it makes more sense to have a separate object for each Resource we can update the issues!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made this change as part of this PR actually, its because when I started working on the git resource I found that having an array in the Spec that refers to another object makes the code too awkward

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay sounds good!

metadata:
name: guestbook-resources-redis-docker
namespace: default
spec:
type: git
params:
- name: url
value: github.com/kubernetes/examples
- name: revision
value: HEAD
value: github.com/GoogleCloudPlatform/redis-docker/
- name: serviceAccount
value: githubServiceAccount
- name: redis-docker
type: git
params:
- name: url
value: github.com/GoogleCloudPlatform/redis-docker/blob/master/4/debian9/4.0/Dockerfile
- name: serviceAccount
value: githubServiceAccount
- name: revision
value: HEAD
- name: stagingRegistry
type: image
params:
- name: url
value: gcr.io/demo-staging
- name: revision
value: HEAD
---
apiVersion: pipeline.knative.dev/v1alpha1
kind: Resource
metadata:
name: guestbookstagingimage
namespace: default
spec:
type: image
params:
- name: url
value: gcr.io/demo-staging
---
apiVersion: pipeline.knative.dev/v1alpha1
kind: Resource
metadata:
name: redisstagingimage
namespace: default
spec:
type: image
params:
- name: url
value: gcr.io/redis-demo-staging
Loading