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 certmanager race condition and version numbers. #1134

Merged
merged 1 commit into from
Apr 30, 2020

Conversation

jlewi
Copy link
Contributor

@jlewi jlewi commented Apr 24, 2020

* To fix the race condition with certmanager (kubeflow#1125) we move the KF
  issuer into a separate package from the package deploying kubeflow.
  This way we can wait for cert-manager to start before deploying resources.

* Labels need to be immutable otherwise upgrades won't work (see kubeflow#1131).
  So remove version number from common labels and application selector
  so that apply will work to update resources.
@kubeflow-bot
Copy link
Contributor

This change is Reviewable

@jlewi
Copy link
Contributor Author

jlewi commented Apr 24, 2020

/assign @krishnadurai

@jlewi
Copy link
Contributor Author

jlewi commented Apr 29, 2020

@krishnadurai @johnugeorge @yanniszark could one of you review this please?

Copy link
Contributor

@krishnadurai krishnadurai left a comment

Choose a reason for hiding this comment

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

/lgtm

@yanniszark
Copy link
Contributor

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: yanniszark

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 f0de832 into kubeflow:master Apr 30, 2020
jlewi pushed a commit to jlewi/manifests that referenced this pull request Jun 1, 2020
* GoogleCloudPlatform/kubeflow-distribution#33 is tracking GCP blueprints on private GKE with VPC-SC

  * This PR doesn't fully enable that but it includes a lot of necessary
    changes.

* cluster-private-patch.yaml is a cluster patch that turns on a lot of
  settings to deploy GKE with private GKE

  * For ease of use we make the master publicly accessible anywhere; users
    could configure that behavior if desired using patch overlays.

* Use kpt setters to name all the networking resources (firewall rules, networks, etc...)

  * This ensures the names are unique based on the KF deployment name and won't conflict with
    existing rules.

  * The setters also ensures that the references get set correctly; e.g. the firewall rules
    correctly refer the newly created network.

* Add a CNRM resource to enable CloudDNS.

  * Per GoogleCloudPlatform/kubeflow-distribution#31 we should probably use CNRM and not AnthosCLI to enable
    all required services.

* Add a kpt setter to control firewall rule logging

  * Enabling firewall rule logging can be useful to debug why connections are blocked.

    Enable logging on firewall rules.

* Add an extra firewall rule for ISTIO

  *Per https://istio.io/docs/setup/platform-setup/gke/ we need to manually create an additional firewall rule to allow traffic to the ISTIO pilot webhook port.

* Add a NAT to allow outbound internet egress

  * Egress is still blocked by firewall rules
  * Per kbueflow/gcp-blueprints#34 this was an attempt to make it possible
    to pull images from DockerHub and Quay.IO. This was partially
    succesful; pulling from DockerHub works but for Quay.IO the firewall
    rules are strill blocking required connections.

* Fix the v3 version of the cert-manager package.

  * kubeflow#1134 moved the kubeflow issuer into its own package to avoid
    race conditions

   * That refactored means that the v3 packages no longer included the
     actual cert-manager resources
   * This PR fixes that by having the v3 package pull in the base package
k8s-ci-robot pushed a commit that referenced this pull request Jun 2, 2020
* GoogleCloudPlatform/kubeflow-distribution#33 is tracking GCP blueprints on private GKE with VPC-SC

  * This PR doesn't fully enable that but it includes a lot of necessary
    changes.

* cluster-private-patch.yaml is a cluster patch that turns on a lot of
  settings to deploy GKE with private GKE

  * For ease of use we make the master publicly accessible anywhere; users
    could configure that behavior if desired using patch overlays.

* Use kpt setters to name all the networking resources (firewall rules, networks, etc...)

  * This ensures the names are unique based on the KF deployment name and won't conflict with
    existing rules.

  * The setters also ensures that the references get set correctly; e.g. the firewall rules
    correctly refer the newly created network.

* Add a CNRM resource to enable CloudDNS.

  * Per GoogleCloudPlatform/kubeflow-distribution#31 we should probably use CNRM and not AnthosCLI to enable
    all required services.

* Add a kpt setter to control firewall rule logging

  * Enabling firewall rule logging can be useful to debug why connections are blocked.

    Enable logging on firewall rules.

* Add an extra firewall rule for ISTIO

  *Per https://istio.io/docs/setup/platform-setup/gke/ we need to manually create an additional firewall rule to allow traffic to the ISTIO pilot webhook port.

* Add a NAT to allow outbound internet egress

  * Egress is still blocked by firewall rules
  * Per kbueflow/gcp-blueprints#34 this was an attempt to make it possible
    to pull images from DockerHub and Quay.IO. This was partially
    succesful; pulling from DockerHub works but for Quay.IO the firewall
    rules are strill blocking required connections.

* Fix the v3 version of the cert-manager package.

  * #1134 moved the kubeflow issuer into its own package to avoid
    race conditions

   * That refactored means that the v3 packages no longer included the
     actual cert-manager resources
   * This PR fixes that by having the v3 package pull in the base package
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

6 participants