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

Kustomize update #1805

Closed
wants to merge 4 commits into from
Closed

Kustomize update #1805

wants to merge 4 commits into from

Conversation

illy
Copy link

@illy illy commented Nov 8, 2021

the current yaml files use some beta version API, which are not supported any more by K8S, in turn, this causes failures when we run Kustomize to install flyte on customized server.

To fix the above issue, we update the apiVersion in the following three yaml files according to https://kubernetes.io/docs/reference/using-api/deprecation-guide/, and successfully deployed to our server.

@welcome
Copy link

welcome bot commented Nov 8, 2021

Thank you for opening this pull request! 🙌
These tips will help get your PR across the finish line: - Most of the repos have a PR template; if not, fill it out to the best of your knowledge. - Sign off your commits (Reference: DCO Guide).

Copy link
Contributor

@EngHabu EngHabu left a comment

Choose a reason for hiding this comment

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

Are these changes backward compatible? will applying the new manifests on an existing deployment break or duplicate the objects?

Comment on lines 20 to 26
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

Comment on lines 10 to 27
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
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

@EngHabu
Copy link
Contributor

EngHabu commented Nov 24, 2021

Please click on Details for "DCO" and follow instructions to resolve it

@EngHabu
Copy link
Contributor

EngHabu commented Dec 10, 2021

Closing for lack of activity. Replacing with #1928

@EngHabu EngHabu closed this Dec 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants