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

Fix cleanup_ci.py - fix cleanup of argo workflows and IAM policy bindings #359

Merged
merged 3 commits into from
Apr 16, 2019

Conversation

jlewi
Copy link
Contributor

@jlewi jlewi commented Apr 15, 2019

Fix cleanup_ci.py - cleaning of Argo and service account binding

  • Add logging to make it easier to monitor progress.

  • Print out stack traces on errors.

  • cleanup_workflows needs to get GKE credentials.

    • The project might be different from the project where KF is deployed.

Related to #358 cleanup ci not cleaning up Argo workflows

Related to #357 cleanup ci not cleaning up IAM policies.

  • Fix bug in cleaning up Argo workflows; we should not break out of loop.

  • Don't exit if delete of Workflow fails; it looks like there might be
    some race conditions.

  • Update the docker image to use google-api-python-client library 1.7.0 instead
    of 1.6.5. It looks like using 1.6.5 gives us an error when getting IAM
    policy.

  • Prune the OWNERs file to remove a bunch of approvers who haven't been active.


This change is Reviewable

* Add logging to make it easier to monitor progress.
* Print out stack traces on errors.

* cleanup_workflows needs to get GKE credentials.
  * The project might be different from the project where KF is deployed.

Related to kubeflow#358 cleanup ci not cleaning up Argo workflows

Related to kubeflow#357 cleanup ci not cleaning up IAM policies.

* Fix bug in cleaning up Argo workflows; we should not break out of loop.

* Don't exit if delete of Workflow fails; it looks like there might be
  some race conditions.

* Update the docker image to use google-api-python-client library 1.7.0 instead
  of 1.6.5. It looks like using 1.6.5 gives us an error when getting IAM
  policy.
@jlewi jlewi removed request for zjj2wry and kunmingg April 15, 2019 20:49
@jlewi
Copy link
Contributor Author

jlewi commented Apr 15, 2019

/assign @gabrielwen

@jlewi jlewi changed the title [WIP] Fix cleanup_ci.py Fix cleanup_ci.py - fix cleanup of argo workflows and IAM policy bindings Apr 15, 2019
Copy link
Contributor

@gabrielwen gabrielwen left a comment

Choose a reason for hiding this comment

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

/lgtm

@@ -21,6 +21,7 @@ RUN set -ex \
wget \
ca-certificates \
git \
emacs \
Copy link
Contributor

Choose a reason for hiding this comment

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

why emacs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Its convenient for debugging; e.g. manually editing of files.

Copy link
Contributor

Choose a reason for hiding this comment

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

ah, ok.

@jlewi
Copy link
Contributor Author

jlewi commented Apr 16, 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

@k8s-ci-robot k8s-ci-robot merged commit 92ffdd0 into kubeflow:master Apr 16, 2019
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.

3 participants