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

Kustomize update #1805

Closed
wants to merge 4 commits into from
Closed

Kustomize update #1805

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6cafcb5
Update wf_crd.yaml
illy Nov 8, 2021
97f9cf3
Update rbac.yaml
illy Nov 8, 2021
216a35c
Update adminserviceaccount.yaml
illy Nov 8, 2021
1d47f29
remove redundant schema as suggested by @EngHabu
illy Nov 15, 2021
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
2 changes: 1 addition & 1 deletion kustomize/base/adminserviceaccount/adminserviceaccount.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ metadata:
namespace: flyte

---
apiVersion: rbac.authorization.k8s.io/v1beta1
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
name: flyteadmin-binding
Expand Down
2 changes: 1 addition & 1 deletion kustomize/base/propeller/rbac.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ metadata:
---
# Create a binding from Role -> ServiceAccount
kind: ClusterRoleBinding
apiVersion: rbac.authorization.k8s.io/v1beta1
apiVersion: rbac.authorization.k8s.io/v1
metadata:
name: flytepropeller
namespace: flyte
Expand Down
20 changes: 18 additions & 2 deletions kustomize/base/wf_crd/wf_crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
apiVersion: apiextensions.k8s.io/v1beta1
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
# name must match the spec fields below, and be in the form: <plural>.<group>
Expand All @@ -7,7 +7,23 @@ spec:
# group name to use for REST API: /apis/<group>/<version>
group: flyte.lyft.com
# version name to use for REST API: /apis/<group>/<version>
version: v1alpha1
versions:
- name: v1alpha1
served: true
storage: true
schema:
openAPIV3Schema:
type: object
properties:
spec:
type: object
properties:
cronSpec:
type: string
image:
type: string
replicas:
type: integer
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we have these properties on the object... this can be left empty. right?

Copy link
Author

Choose a reason for hiding this comment

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

We are not quite familiar with this part. However, if we don't include anything in the schema, it still broke. That's why we included an empty schema. If you think this can be left empty, please go ahead.

Copy link
Contributor

Choose a reason for hiding this comment

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

this worked for me:

  versions:
  - name: v1alpha1
    schema:
      openAPIV3Schema:
        description: 'FlyteWorkflow: represents one Execution Workflow object'
        type: object
    served: true
    storage: true

# either Namespaced or Cluster
Copy link
Contributor

Choose a reason for hiding this comment

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

This worked for me

Suggested change
versions:
- name: v1alpha1
served: true
storage: true
schema:
openAPIV3Schema:
type: object
properties:
spec:
type: object
properties:
cronSpec:
type: string
image:
type: string
replicas:
type: integer
# either Namespaced or Cluster
versions:
- name: v1alpha1
schema:
openAPIV3Schema:
description: 'FlyteWorkflow: represents one Execution Workflow object'
type: object
served: true
storage: true
# either Namespaced or Cluster

Copy link
Author

Choose a reason for hiding this comment

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

thanks @EngHabu , i tested in my laptop, and it worked.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for confirming. Can you please update the PR so I can approve and merge?

Copy link
Author

Choose a reason for hiding this comment

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

updated the changes, thanks again

names:
# plural name to be used in the URL: /apis/<group>/<version>/<plural>
Expand Down