Skip to content

Add missing files for k8s_istio.v1.1.0 KFDef#1515

Merged
k8s-ci-robot merged 2 commits into
kubeflow:masterfrom
pvaneck:kfdef-k8s
Aug 31, 2020
Merged

Add missing files for k8s_istio.v1.1.0 KFDef#1515
k8s-ci-robot merged 2 commits into
kubeflow:masterfrom
pvaneck:kfdef-k8s

Conversation

@pvaneck

@pvaneck pvaneck commented Aug 25, 2020

Copy link
Copy Markdown
Member

Description of your changes:
Running through install instructions for using the kfctl_k8s_istio.v1.1.0.yaml KFDef file, I was running into issues with some manifests missing. This PR creates those missing files, following the current paradigm of having these exist in the stacks/kubernetes/application directory. Once I added these, Kubeflow deployed successfully.

Also removed Tensorboard from the KFDef. (see #415)

Can generate tests if these changes make sense.

Checklist:

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

@pvaneck
Add missing files for k8s_istio.v1.1.0 KFDef
a4f3b23 
* Also removed tensorboard from the KFDef
@google-cla google-cla Bot added the cla: yes label Aug 25, 2020
@kubeflow-bot

kubeflow-bot commented Aug 25, 2020

Copy link
Copy Markdown
Contributor

This change is Reviewable

@k8s-ci-robot

k8s-ci-robot commented Aug 25, 2020

Copy link
Copy Markdown
Contributor

Hi @pvaneck. Thanks for your PR.

I'm waiting for a kubeflow member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

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.

@PatrickXYS

PatrickXYS commented Aug 25, 2020

Copy link
Copy Markdown
Member

It makes sense to me, can you generate tests?

@pvaneck

PatrickXYS
PatrickXYS suggested changes Aug 25, 2020
View reviewed changes
Comment thread kfdef/kfctl_k8s_istio.v1.1.0.yaml
name: manifests
path: stacks/kubernetes/application/spartakus
name: spartakus
- kustomizeConfig:

@PatrickXYS PatrickXYS Aug 25, 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.

If we remove tensorboard from manifests, can you also remove folders stacks/kubernetes/application/tensorboard?

@pvaneck pvaneck Aug 25, 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.

stacks/kubernetes/application/tensorboard never seemed to have existed, so we should be good there. :)

@PatrickXYS

PatrickXYS commented Aug 25, 2020

Copy link
Copy Markdown
Member

/test kubeflow-manifests-presubmit

@PatrickXYS

PatrickXYS commented Aug 27, 2020

Copy link
Copy Markdown
Member

/lgtm

krishnadurai
krishnadurai approved these changes Aug 28, 2020
View reviewed changes

@krishnadurai krishnadurai left a comment

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.

/lgtm
/approve

@k8s-ci-robot

k8s-ci-robot commented Aug 28, 2020

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: krishnadurai

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

@swiftdiaries

swiftdiaries commented Aug 31, 2020

Copy link
Copy Markdown
Member

/retest

@swiftdiaries swiftdiaries mentioned this pull request Aug 31, 2020
Closed
@k8s-ci-robot k8s-ci-robot merged commit ed61541 into kubeflow:master Aug 31, 2020
pvaneck added a commit to pvaneck/manifests that referenced this pull request Sep 1, 2020
@pvaneck
Add missing files for k8s_istio.v1.1.0 KFDef (kubeflow#1515)
8cc230f 
* Add missing files for k8s_istio.v1.1.0 KFDef

* Also removed tensorboard from the KFDef

* Generate tests
swiftdiaries added a commit to swiftdiaries/manifests that referenced this pull request Sep 2, 2020
@swiftdiaries
Add missing files for k8s_istio.v1.1.0 KFDef (kubeflow#1515)
887f14f 
* Add missing files for k8s_istio.v1.1.0 KFDef

* Also removed tensorboard from the KFDef

* Generate tests
k8s-ci-robot pushed a commit that referenced this pull request Sep 4, 2020
@swiftdiaries
cherry-pick k8s_istio.v1.1.0 KFDef (#1515) (#1540)
98fa8c8 
* Add missing files for k8s_istio.v1.1.0 KFDef

* Also removed tensorboard from the KFDef

* Generate tests
animeshsingh added a commit to animeshsingh/manifests that referenced this pull request Nov 11, 2020
@animeshsingh
Add Paul as a reviewer
ec58f7d 
Will be responsible for maintaining manifests for KFServing, OpenShift, IKS etc.

PRs merged: 
kubeflow#1515
kubeflow#1575
kubeflow#1580
kubeflow#1617

Reviews:
kubeflow#1582
kubeflow#1621
kubeflow#1625
kubeflow#1627
kubeflow#1628
kubeflow/kfctl#419
@animeshsingh animeshsingh mentioned this pull request Nov 11, 2020
Merged
k8s-ci-robot pushed a commit that referenced this pull request Nov 11, 2020
@animeshsingh
Add Paul as a reviewer (#1632)
fcd557a 
* Add Paul as a reviewer

Will be responsible for maintaining manifests for KFServing, OpenShift, IKS etc.

PRs merged: 
#1515
#1575
#1580
#1617

Reviews:
#1582
#1621
#1625
#1627
#1628
kubeflow/kfctl#419

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

Reviewers

2 more reviewers

@PatrickXYS PatrickXYS PatrickXYS requested changes

@krishnadurai krishnadurai krishnadurai approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@krishnadurai krishnadurai

@PatrickXYS PatrickXYS

Labels

approved cla: yes lgtm needs-ok-to-test size/XXL

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@pvaneck @kubeflow-bot @k8s-ci-robot @PatrickXYS @swiftdiaries @krishnadurai