Skip to content

Migrate AWS manifests to v3 pattern#1396

Merged
k8s-ci-robot merged 4 commits into
kubeflow:masterfrom
Jeffwan:aws_v3
Jul 21, 2020
Merged

Migrate AWS manifests to v3 pattern#1396
k8s-ci-robot merged 4 commits into
kubeflow:masterfrom
Jeffwan:aws_v3

Conversation

@Jeffwan

@Jeffwan Jeffwan commented Jul 20, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

Which issue is resolved by this Pull Request:
Resolves #1095

Description of your changes:
Get some time to migrate AWS manifest to v3 pattern. I will have an update kfdef PR coming later.

Checklist:

  • Unit tests have been rebuilt:
    1. cd manifests/tests
    2. make generate-changed-only
    3. make test

@google-cla google-cla Bot added the cla: yes label Jul 20, 2020
@kubeflow-bot

kubeflow-bot commented Jul 20, 2020 •
edited by jlewi
Loading

Copy link
Copy Markdown
Contributor

This change is Reviewable

@Jeffwan

Jeffwan commented Jul 20, 2020

Copy link
Copy Markdown
Member Author

/cc @jlewi @PatrickXYS

@k8s-ci-robot k8s-ci-robot requested review from PatrickXYS and jlewi July 20, 2020 08:37
@jlewi

jlewi commented Jul 20, 2020

Copy link
Copy Markdown
Contributor

@Jeffwan Is there a co-maintainer of the AWS manifests who could review this PR? Maybe @ellistarn ?

@Jeffwan

Jeffwan commented Jul 20, 2020

Copy link
Copy Markdown
Member Author

@jlewi @PatrickXYS will review this PR but we don't have permission to leave approve since this is a new folder. He will leave lgtm after review. It adds a new OWNER files under stacks/aws and we can approve ourselves later.

PatrickXYS
PatrickXYS reviewed Jul 20, 2020
View reviewed changes
Comment thread aws/istio-ingress/base_v3/kustomization.yaml
@@ -0,0 +1,22 @@
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization

@PatrickXYS PatrickXYS Jul 20, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we also need to add namespace: istio-system, the same as what you did in aws/istio-ingress/base/kustomization.yaml file.

@PatrickXYS PatrickXYS Jul 20, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Other than this, lgtm

@Jeffwan Jeffwan Jul 20, 2020

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yeah. I think we should explicitly list here. Will do

@kubeflow kubeflow deleted a comment from kubeflow-bot Jul 20, 2020
@kubeflow kubeflow deleted a comment from kubeflow-bot Jul 20, 2020
@PatrickXYS

PatrickXYS commented Jul 20, 2020

Copy link
Copy Markdown
Member

/lgtm

@PatrickXYS

PatrickXYS commented Jul 20, 2020

Copy link
Copy Markdown
Member

/test kubeflow-manifests-presubmit

@k8s-ci-robot k8s-ci-robot removed the lgtm label Jul 20, 2020
@PatrickXYS

PatrickXYS commented Jul 20, 2020

Copy link
Copy Markdown
Member

/lgtm

@PatrickXYS

PatrickXYS commented Jul 20, 2020

Copy link
Copy Markdown
Member

/cc @jlewi can you approve the PR since we have reviewed and tests passed?

@PatrickXYS

PatrickXYS commented Jul 20, 2020

Copy link
Copy Markdown
Member

/cc @terrytangyuan @IronPan @yanniszark Can you take a look if possible?

@PatrickXYS

PatrickXYS commented Jul 20, 2020

Copy link
Copy Markdown
Member

Seems like we haven't updated files like kfctl_aws.yaml

@Jeffwan

Jeffwan commented Jul 20, 2020 •
edited
Loading

Copy link
Copy Markdown
Member Author

es like kfctl_aws.yaml

It could be in a separate PR.

terrytangyuan
terrytangyuan approved these changes Jul 21, 2020
View reviewed changes

@terrytangyuan terrytangyuan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

/approve

@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: terrytangyuan

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

@k8s-ci-robot k8s-ci-robot merged commit 94d1e48 into kubeflow:master Jul 21, 2020
@Jeffwan Jeffwan deleted the aws_v3 branch July 21, 2020 04:26
PatrickXYS pushed a commit to PatrickXYS/manifests that referenced this pull request Jul 24, 2020
Migrate AWS manifests to v3 pattern (kubeflow#1396)
8bdefec 
* Migrate AWS manifest to v3 pattern

* Clean up tests files

* Add istio namespace to istio ingress

* Update KFP pipeline test case for aws stack
k8s-ci-robot pushed a commit that referenced this pull request Jul 25, 2020
Cherry pick #1396 #1410 to v1.1-branch (#1419)
d8be7f9 
* Migrate AWS manifests to v3 pattern (#1396)

* Migrate AWS manifest to v3 pattern

* Clean up tests files

* Add istio namespace to istio ingress

* Update KFP pipeline test case for aws stack

* AWS Kfdef changes using v3 stacks  (#1410)

* Changes needed for V3 change on AWS

* Adopt AWS ECR image for jupyter-web-app components and generate all tests against manifest V3 changes

* Adopt Cognito change against V3

* Adopt change for multi-user KFP

* Update tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gaocegege gaocegege Awaiting requested review from gaocegege

@krishnadurai krishnadurai Awaiting requested review from krishnadurai

@jlewi jlewi Awaiting requested review from jlewi

2 more reviewers

@PatrickXYS PatrickXYS PatrickXYS left review comments

@terrytangyuan terrytangyuan terrytangyuan approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@PatrickXYS PatrickXYS

Labels

approved cla: yes lgtm size/XXL

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

v3 version of AWS KFDefs

6 participants

@Jeffwan @kubeflow-bot @jlewi @PatrickXYS @k8s-ci-robot @terrytangyuan