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

CD scripts for updating kubeflow manifests should close older PRs #571

Closed
jlewi opened this issue Jan 22, 2020 · 4 comments
Closed

CD scripts for updating kubeflow manifests should close older PRs #571

jlewi opened this issue Jan 22, 2020 · 4 comments

Comments

@jlewi
Copy link
Contributor

jlewi commented Jan 22, 2020

Follow on to #450

We have a server which continually opens up PRs to update the Kubeflow manifests to a newly built docker image for each application
https://github.com/kubeflow/testing/tree/master/apps-cd

Its not uncommon for us to end up with multiple PRs in flight for a given application because there were multiple commits to an application.

In this case, we should automatically close all but the most recent PR for an application so as not to confuse the reviewer.

@issue-label-bot
Copy link

Issue-Label Bot is automatically applying the labels:

Label Probability
feature 0.85

Please mark this comment with 👍 or 👎 to give our bot feedback!
Links: app homepage, dashboard and code for this bot.

@jlewi
Copy link
Contributor Author

jlewi commented Jan 23, 2020

It looks like the hub CLI doesn't support closing PRs so we will need to use the GitHub API to do this.

I think we will want to create a python script which uses a GitHub App to comment and close PRs.

jlewi pushed a commit to jlewi/testing that referenced this issue Jan 24, 2020
* In the event there are multiple PRs open to update a Kubeflow application
  we want to close the older PRs; so there is a single open PR updating
  the application to the newest code.

Related to kubeflow#571
jlewi pushed a commit to jlewi/testing that referenced this issue Jan 28, 2020
* In the event there are multiple PRs open to update a Kubeflow application
  we want to close the older PRs; so there is a single open PR updating
  the application to the newest code.

Related to kubeflow#571
jlewi pushed a commit to jlewi/testing that referenced this issue Jan 28, 2020
* In the event there are multiple PRs open to update a Kubeflow application
  we want to close the older PRs; so there is a single open PR updating
  the application to the newest code.

Related to kubeflow#571
jlewi pushed a commit to jlewi/testing that referenced this issue Jan 28, 2020
* In the event there are multiple PRs open to update a Kubeflow application
  we want to close the older PRs; so there is a single open PR updating
  the application to the newest code.

  Related to kubeflow#571

* Setup a def namespace for use with apps-cd.

* Update update_kf_apps.py to close old PRs on each sync.

* Bake the source code into the docker image rather than using a wrapper
  script to sync the code from git.

  * Sync'ing the code from git became to difficult to reason about once
    we start splitting the source code across multiple repositories
    * We now depend on github/kubeflow/code-intelligence for utilities
      for working with GitHub Apps

    * Using a docker image also ensures we don't get broken suddenly when
      new changes are in place

    * In the future we could use github actions to automate updating the
      deployment on postsubmits

* Turn app-pipeline.template.yaml into a ConfigMap
  * This allows better versioning
  * We can rely on kustomize to create a configmap with a hash based on the
    contents
  * kustomize will then reference the config map using its hash. As
    a result a rolling update is triggered whenever the hash contents changes.
  * This makes it easier to handle rollous and updates.

Define a dev instance of the update KF apps infrastructure to facilitate development
  * Use profiles in skaffold.

  * update_kf_apps.py in dev uses a config map now to ubtain app-pipeline.template.yaml
    rather than fetching it from git

  * This makes it much easier to test changes in the dev instance

Fix a bunch of bugs preventing update_kf_apps.py from working
   * Update requirements.txt with a bunch of missing packages.
   * Fix some imports in update_kf_apps.py

  * Need to set resource requests for the build pods otherwise builds get
    CPU starved and take forever.

Miscellaneous

* Create a tool to copy secrets between namespaces from GCS
k8s-ci-robot pushed a commit that referenced this issue Jan 28, 2020
* Define a script to close obsolete PRs to update an application.

* In the event there are multiple PRs open to update a Kubeflow application
  we want to close the older PRs; so there is a single open PR updating
  the application to the newest code.

Related to #571

* Define a script to close obsolete PRs to update an application.

* In the event there are multiple PRs open to update a Kubeflow application
  we want to close the older PRs; so there is a single open PR updating
  the application to the newest code.

  Related to #571

* Setup a def namespace for use with apps-cd.

* Update update_kf_apps.py to close old PRs on each sync.

* Bake the source code into the docker image rather than using a wrapper
  script to sync the code from git.

  * Sync'ing the code from git became to difficult to reason about once
    we start splitting the source code across multiple repositories
    * We now depend on github/kubeflow/code-intelligence for utilities
      for working with GitHub Apps

    * Using a docker image also ensures we don't get broken suddenly when
      new changes are in place

    * In the future we could use github actions to automate updating the
      deployment on postsubmits

* Turn app-pipeline.template.yaml into a ConfigMap
  * This allows better versioning
  * We can rely on kustomize to create a configmap with a hash based on the
    contents
  * kustomize will then reference the config map using its hash. As
    a result a rolling update is triggered whenever the hash contents changes.
  * This makes it easier to handle rollous and updates.

Define a dev instance of the update KF apps infrastructure to facilitate development
  * Use profiles in skaffold.

  * update_kf_apps.py in dev uses a config map now to ubtain app-pipeline.template.yaml
    rather than fetching it from git

  * This makes it much easier to test changes in the dev instance

Fix a bunch of bugs preventing update_kf_apps.py from working
   * Update requirements.txt with a bunch of missing packages.
   * Fix some imports in update_kf_apps.py

  * Need to set resource requests for the build pods otherwise builds get
    CPU starved and take forever.

Miscellaneous

* Create a tool to copy secrets between namespaces from GCS

* Fix lint.

* Due to #460 we need to disable pylint.
@stale
Copy link

stale bot commented Apr 23, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in one week if no further activity occurs. Thank you for your contributions.

@stale
Copy link

stale bot commented Apr 30, 2020

This issue has been closed due to inactivity.

@stale stale bot closed this as completed Apr 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant