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

Add kfctl E2E test on GCP with IAP #2814

Merged
merged 6 commits into from Mar 28, 2019
Merged

Conversation

jlewi
Copy link
Contributor

@jlewi jlewi commented Mar 26, 2019

Related to #2610 E2E test for go binary


This change is Reviewable

@jlewi
Copy link
Contributor Author

jlewi commented Mar 26, 2019

#2795 should be submitted first.

@jlewi
Copy link
Contributor Author

jlewi commented Mar 27, 2019

Most recent failure was problem creating sandbox: kubeflow/testing#342

@jlewi
Copy link
Contributor Author

jlewi commented Mar 27, 2019

/test all

@jlewi jlewi changed the title [WIP] Add kfctl E2E test on GCP with IAP Add kfctl E2E test on GCP with IAP Mar 27, 2019
@jlewi
Copy link
Contributor Author

jlewi commented Mar 27, 2019

Test passed!

Related to: kubeflow#2610 E2E test for kfctl.

* Make test names unique based on parameters.
@jlewi
Copy link
Contributor Author

jlewi commented Mar 27, 2019

/assign @gabrielwen
/assign @kkasravi

@jlewi
Copy link
Contributor Author

jlewi commented Mar 27, 2019

@gabrielwen @kkasravi This should be ready to review.

@@ -71,10 +80,9 @@ def test_build_kfctl_go(app_path, project):
# username and password are passed as env vars and won't appear in the logs
run_with_retries([kfctl_path, "init", app_path, "-V", "--platform=gcp",
"--version=" + version,
Copy link
Contributor

Choose a reason for hiding this comment

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

should the PULL_NUMBER be used to fetch that PR from github
eg `kfctl init ... --version pull/os.getenv("PULL_NUMBER")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Isn't that what's happening?
Although once #2831 we should just pull the configs from the local checkout of the source.

init_args = ["--use_basic_auth"]
else:
# Owned by project kubeflow-ci-deployment.
os.environ["CLIENT_ID"] = "CJ4qVPLTi0j0GJMkONj7Quwt"
Copy link
Contributor

Choose a reason for hiding this comment

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

is it safe to put these into source control?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. CLIENT id's can only be used with specific URLs which we own and configure.
CLIENT_ID & SECRET aren't actually secret. e.g. if you use them in a webapp in client side JS they are visible.

@jlewi
Copy link
Contributor Author

jlewi commented Mar 27, 2019

Latest test failure is:

INFO Error: couldn't apply KfApp: coordinator Apply failed for gcp: gcp apply could not update deployment manager Error could not update cluster-kubeflow.yaml: Update deployment error: googleapi: Error 409: Resource 'projects/kubeflow-ci-deployment/global/deployments/kfctl-e58c' has an ongoing conflicting operation: 'projects/kubeflow-ci-deployment/global/operations/operation-1553727977511-5851b7c2d5878-c8dabac2-0ec03ba6'., conflict

But it looks like the pod actually got evicted because of the deadline.

Pod was active on the node longer than the specified deadline

@jlewi
Copy link
Contributor Author

jlewi commented Mar 28, 2019

Woo Hoo! Tests are passing again. I had to increase the timeout.

@gabrielwen
Copy link
Contributor

Woo Hoo! Tests are passing again. I had to increase the timeout.

yea... timeout is becoming an issue on my end as well...

@jlewi
Copy link
Contributor Author

jlewi commented Mar 28, 2019

@gabrielwen could you LGTM please?

@gabrielwen
Copy link
Contributor

/lgtm

@jlewi
Copy link
Contributor Author

jlewi commented Mar 28, 2019

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jlewi

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

The pull request process is described here

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
Copy link
Contributor Author

jlewi commented Mar 28, 2019

Recent test should be a flake.

util.py                     71 INFO     Error: couldn't delete KfApp: coordinator Delete failed for gcp: Error when cleaning IAM policy: googleapi: Error
409: There were concurrent policy changes. Please retry the whole read-modify-write with exponential backoff., aborted

/test all

@k8s-ci-robot k8s-ci-robot merged commit c44e95b into kubeflow:master Mar 28, 2019
lluunn pushed a commit to lluunn/kubeflow that referenced this pull request Mar 28, 2019
* Add an E2E test for kfctl with IAP on GCP.

Related to: kubeflow#2610 E2E test for kfctl.

* Make test names unique based on parameters.

* Revert to pulling HEAD.

* Fix comment.

* Set requests & limits.

* Increase timeout.

* Add a comment.
saffaalvi pushed a commit to StatCan/kubeflow that referenced this pull request Feb 11, 2021
* Add an E2E test for kfctl with IAP on GCP.

Related to: kubeflow#2610 E2E test for kfctl.

* Make test names unique based on parameters.

* Revert to pulling HEAD.

* Fix comment.

* Set requests & limits.

* Increase timeout.

* Add a comment.
saffaalvi pushed a commit to StatCan/kubeflow that referenced this pull request Feb 12, 2021
* Add an E2E test for kfctl with IAP on GCP.

Related to: kubeflow#2610 E2E test for kfctl.

* Make test names unique based on parameters.

* Revert to pulling HEAD.

* Fix comment.

* Set requests & limits.

* Increase timeout.

* Add a comment.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants