Skip to content

Convert v1 to v2 setters & substituions in gcp#1398

Merged
k8s-ci-robot merged 5 commits into
kubeflow:masterfrom
jlewi:blueprints
Jul 21, 2020
Merged

Convert v1 to v2 setters & substituions in gcp#1398
k8s-ci-robot merged 5 commits into
kubeflow:masterfrom
jlewi:blueprints

Conversation

@jlewi

@jlewi jlewi commented Jul 20, 2020

Copy link
Copy Markdown
Contributor

  • The latest version of kpt started choking on gcp/v2 because we were
    still using the old style setters and substitutions.

  • This PR creates a kptfile to use the new setter and substitutions.

  • hack/create_kptfile.py contains a script to generate lot of the setters
    and substitutions.

  • kf-vm-sa.yaml shouldn't specify the namespace; this will get set in an overlay

  • Move workload identity bindings for kf-admin KSA from kubeflow/instance in gcp blueprints repo into this repository.

related to gcp-blueprints#89

Convert v1 to v2 setters & substituions in gcp
806973a 
* The latest version of kpt started choking on gcp/v2 because we were
  still using the old style setters and substitutions.

* This PR creates a kptfile to use the new setter and substitutions.

* hack/create_kptfile.py contains a script to generate lot of the setters
  and substitutions.

* kf-vm-sa.yaml shouldn't specify the namespace; this will get set in an overlay

* Move workload identity bindings for kf-admin KSA from kubeflow/instance in gcp blueprints repo into this repository.

related to gcp-blueprints#89
@google-cla google-cla Bot added the cla: yes label Jul 20, 2020
@kubeflow-bot

kubeflow-bot commented Jul 20, 2020

Copy link
Copy Markdown
Contributor

This change is Reviewable

@jlewi

jlewi commented Jul 20, 2020

Copy link
Copy Markdown
Contributor Author

/assign @Bobgy

If you look at the diff for testdata_gcp_kpt you can see that the files are mostly unchanged indicating that the substitutions match what we had before.

The exception is one or two bug fixes; e.g.

  • There are a couple places where I we were using zone when we should have used location
  • I created separate setters for the management cluster whereas before they were sharing the same setters as the non management cluster.

jlewi pushed a commit to jlewi/gcp-blueprints that referenced this pull request Jul 20, 2020
Convert setters from v1 to v2
a007751 
* Get rid of enable-services.yaml as we are now using CNRM and this is
  in the base package.
* Related to GoogleCloudPlatform#89

* Remove cluster and iam policy patch; these are now in the base manifests.
  Added in kubeflow/community-distribution#1398

* kubeflow/hack/create_kptfile.py contains most of the commands
  used to generate the setters and substitutions.
@jlewi jlewi mentioned this pull request Jul 20, 2020
Merged
@jlewi

jlewi commented Jul 20, 2020

Copy link
Copy Markdown
Contributor Author

/assign @Ark-kun

@Ark-kun Could you review this since its 10PM in shanghai?

@Bobgy

Bobgy commented Jul 21, 2020

Copy link
Copy Markdown
Contributor

/lgtm
/approve

Approving first, going to take a deeper look

Bobgy
Bobgy reviewed Jul 21, 2020
View reviewed changes
Comment thread hack/create_kptfile.py
Comment on lines +1 to +4
"""This is a script created for the updating of the GCP Kpt packages.

The purpose of this is to upgrade to using setters and substitutions
"""

@Bobgy Bobgy Jul 21, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we just use this for upgrade?
How should users create new substitutions when working with blueprint?

@jlewi jlewi Jul 21, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is a onetime script. We can delete it in a subsequent PR. If we need to create additional substitutions we can do so using the kpt CLI.

When doing these types of conversions its just a good practice IMO to check in a script so we have a record in case we need to redo it.

@jlewi

jlewi commented Jul 21, 2020

Copy link
Copy Markdown
Contributor Author

/lgtm
/approve

@k8s-ci-robot

k8s-ci-robot commented Jul 21, 2020

Copy link
Copy Markdown
Contributor

@jlewi: you cannot LGTM your own PR.

Details

In response to this:

/lgtm
/approve

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot

k8s-ci-robot commented Jul 21, 2020

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Bobgy, jlewi

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

jlewi pushed a commit to jlewi/gcp-blueprints that referenced this pull request Jul 21, 2020
Convert setters from v1 to v2
88a0cac 
* Get rid of enable-services.yaml as we are now using CNRM and this is
  in the base package.
* Related to GoogleCloudPlatform#89

* Remove cluster and iam policy patch; these are now in the base manifests.
  Added in kubeflow/community-distribution#1398

* kubeflow/hack/create_kptfile.py contains most of the commands
  used to generate the setters and substitutions.
@k8s-ci-robot k8s-ci-robot merged commit 78c961a into kubeflow:master Jul 21, 2020
k8s-ci-robot pushed a commit to GoogleCloudPlatform/kubeflow-distribution that referenced this pull request Jul 21, 2020
@jlewi
Convert setters from v1 to v2 (#95)
d574d92 
* Get rid of enable-services.yaml as we are now using CNRM and this is
  in the base package.
* Related to #89

* Remove cluster and iam policy patch; these are now in the base manifests.
  Added in kubeflow/community-distribution#1398

* kubeflow/hack/create_kptfile.py contains most of the commands
  used to generate the setters and substitutions.
jlewi pushed a commit to jlewi/gcp-blueprints that referenced this pull request Jul 21, 2020
Convert setters from v1 to v2
72cf098 
* Get rid of enable-services.yaml as we are now using CNRM and this is
  in the base package.
* Related to GoogleCloudPlatform#89

* Remove cluster and iam policy patch; these are now in the base manifests.
  Added in kubeflow/community-distribution#1398

* kubeflow/hack/create_kptfile.py contains most of the commands
  used to generate the setters and substitutions.
jlewi pushed a commit to jlewi/gcp-blueprints that referenced this pull request Jul 21, 2020
Convert setters from v1 to v2
2cf3d49 
* Get rid of enable-services.yaml as we are now using CNRM and this is
  in the base package.
* Related to GoogleCloudPlatform#89

* Remove cluster and iam policy patch; these are now in the base manifests.
  Added in kubeflow/community-distribution#1398

* kubeflow/hack/create_kptfile.py contains most of the commands
  used to generate the setters and substitutions.
k8s-ci-robot pushed a commit that referenced this pull request Jul 21, 2020
@jlewi
Automated cherry pick of #1393: Check in expected kpt output for Kptf…
2d6cd12 
…ile refactoring #1398: Convert v1 to v2 setters & substituions in gcp Cherry pick of #1393 #1398 on v1.1-branch. #1393: Check in expected kpt output for Kptfile refactoring #1398: Convert v1 to v2 setters & substituions in gcp (#1401)

* Check in expected kpt output for Kptfile refactoring

* Per GoogleCloudPlatform/kubeflow-distribution#89 we need to get rid of the legacy
  partial setters and move to using a KptFile and substitutions.

* In preparation for that we want to check in a set of test data
  that is the result of running our kpt cfg set with a given set of
  values

* This test data will be used to verify that the refactoring to use
  a KptFile doesn't change the output.

* After adding the KptFile we can simply regenerate the testdata
  and then look at the diff to ensure there are no unexpected changes.

* Convert v1 to v2 setters & substituions in gcp

* The latest version of kpt started choking on gcp/v2 because we were
  still using the old style setters and substitutions.

* This PR creates a kptfile to use the new setter and substitutions.

* hack/create_kptfile.py contains a script to generate lot of the setters
  and substitutions.

* kf-vm-sa.yaml shouldn't specify the namespace; this will get set in an overlay

* Move workload identity bindings for kf-admin KSA from kubeflow/instance in gcp blueprints repo into this repository.

related to gcp-blueprints#89

* Fix image mirror substitution.

* Create a KptFile for stacks.

* Add conversion for stacks.

* Add KptFile for stacks/gcp
k8s-ci-robot pushed a commit to GoogleCloudPlatform/kubeflow-distribution that referenced this pull request Jul 21, 2020
@jlewi
Automated cherry pick of #95: Convert setters from v1 to v2 Cherry pi…
a23350c 
…ck of #95 on v1.1-branch. #95: Convert setters from v1 to v2 (#97)

* Convert setters from v1 to v2

* Get rid of enable-services.yaml as we are now using CNRM and this is
  in the base package.
* Related to #89

* Remove cluster and iam policy patch; these are now in the base manifests.
  Added in kubeflow/community-distribution#1398

* kubeflow/hack/create_kptfile.py contains most of the commands
  used to generate the setters and substitutions.

* Disable presubmit to deploy Kubeflow.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@krishnadurai krishnadurai Awaiting requested review from krishnadurai

1 more reviewer

@Bobgy Bobgy Bobgy left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@Ark-kun Ark-kun

@Bobgy Bobgy

Labels

approved cla: yes lgtm size/XXL

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@jlewi @kubeflow-bot @Bobgy @k8s-ci-robot @Ark-kun