Skip to content

Commit

Permalink
commonLabels need to be immutable to support upgrading. (#1140)
Browse files Browse the repository at this point in the history
* Fix #1131

* kustomize commonLabels get subsituted into selector fields. Selector fields
  are immutable. So if commonLabels change (e.g. between versions) then
  we can't reapply/update the existing resources which breaks upgrades
 (kubeflow/kfctl#304)

* For the most part the problematic commonLabels were on our Application
  resources. The following labels were being set

  "app.kubernetes.io/version"
  "app.kubernetes.io/instance"
  "app.kubernetes.io/managed-by"
  "app.kubernetes.io/part-of"

* Version was definetely changing between versions. instance was also changing
  between versions to include the version number.

* managed-by and part-of could also change (e.g. we may not be using kfctl)
* We could still set these labels if we wanted to; we just shouldn't set
  them as commonLabels and/or include them in the selector as the will
  inhibit upgrades with kubectl apply.

* I created a test validate_resources_test.go to ensure none of these
  labels are included in commonLabels

* I created a simple go binary tools/fix_common_labels.go to update
  all the resources.

* generat_tests.py - Delete the code to remove unmatched tests.
  * We no longer generate tests that way and the delete code was going
    to delete valid tests like our new validation test

* Get rid of the clean rule in the Makefile for the same reason.
  • Loading branch information
jlewi authored May 1, 2020
1 parent f0de832 commit 4307f37
Show file tree
Hide file tree
Showing 179 changed files with 446 additions and 1,133 deletions.
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
bases:
- ../../base
resources:
- application.yaml
commonLabels:
app.kubernetes.io/name: bootstrap
app.kubernetes.io/instance: bootstrap-v0.7.0
app.kubernetes.io/managed-by: kfctl
app.kubernetes.io/component: bootstrap
app.kubernetes.io/part-of: kubeflow
app.kubernetes.io/version: v0.7.0
app.kubernetes.io/name: bootstrap
kind: Kustomization
resources:
- application.yaml
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
bases:
- ../../base
resources:
- application.yaml
commonLabels:
app.kubernetes.io/name: webhook
app.kubernetes.io/instance: webhook-v0.7.0
app.kubernetes.io/managed-by: kfctl
app.kubernetes.io/component: webhook
app.kubernetes.io/part-of: kubeflow
app.kubernetes.io/version: v0.7.0
app.kubernetes.io/name: webhook
kind: Kustomization
resources:
- application.yaml
14 changes: 3 additions & 11 deletions admission-webhook/webhook/v3/kustomization.yaml
Original file line number Diff line number Diff line change
@@ -1,16 +1,8 @@
# TODO(https://github.com/kubeflow/manifests/issues/1052): Refactor
# and cleanup the kustomization once the v3 migration is done.
apiVersion: kustomize.config.k8s.io/v1beta1
commonLabels:
app.kubernetes.io/component: poddefaults
app.kubernetes.io/name: poddefaults
kind: Kustomization
resources:
# With v3 we want to always use cert-manager to get the
# self-signed certificate
- ../overlays/cert-manager/
- ../overlays/application/application.yaml
commonLabels:
app.kubernetes.io/name: poddefaults
app.kubernetes.io/instance: poddefaults-v0.7.0
app.kubernetes.io/managed-by: kfctl
app.kubernetes.io/component: poddefaults
app.kubernetes.io/part-of: kubeflow
app.kubernetes.io/version: v0.7.0
12 changes: 4 additions & 8 deletions application/application/overlays/application/kustomization.yaml
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
bases:
- ../../base
resources:
- application.yaml
commonLabels:
app.kubernetes.io/name: kubeflow
app.kubernetes.io/instance: kubeflow-v0.7.0
app.kubernetes.io/managed-by: kfctl
app.kubernetes.io/component: kubeflow
app.kubernetes.io/part-of: kubeflow
app.kubernetes.io/version: v0.7.0
app.kubernetes.io/name: kubeflow
kind: Kustomization
resources:
- application.yaml
12 changes: 4 additions & 8 deletions argo/overlays/application/kustomization.yaml
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
bases:
- ../../base
resources:
- application.yaml
commonLabels:
app.kubernetes.io/name: argo
app.kubernetes.io/instance: argo-v2.3.0
app.kubernetes.io/managed-by: kfctl
app.kubernetes.io/component: argo
app.kubernetes.io/part-of: kubeflow
app.kubernetes.io/version: v2.3.0
app.kubernetes.io/name: argo
kind: Kustomization
resources:
- application.yaml
Original file line number Diff line number Diff line change
@@ -1,13 +1,8 @@
apiVersion: kustomize.config.k8s.io/v1beta1
commonLabels:
app.kubernetes.io/component: aws-alb-ingress-controller
app.kubernetes.io/name: aws-alb-ingress-controller
kind: Kustomization
resources:
- ../../base
- application.yaml
commonLabels:
app.kubernetes.io/name: aws-alb-ingress-controller
app.kubernetes.io/instance: aws-alb-ingress-controller-v0.7.0
app.kubernetes.io/managed-by: kfctl
app.kubernetes.io/component: aws-alb-ingress-controller
app.kubernetes.io/part-of: kubeflow
app.kubernetes.io/version: v0.7.0

Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
bases:
- ../../base
resources:
- application.yaml
commonLabels:
app.kubernetes.io/name: aws-istio-authz-adaptor
app.kubernetes.io/instance: aws-istio-authz-adaptor-0.1
app.kubernetes.io/managed-by: kfctl
app.kubernetes.io/component: aws-istio-authz-adaptor
app.kubernetes.io/part-of: kubeflow
app.kubernetes.io/version: "0.1"
app.kubernetes.io/name: aws-istio-authz-adaptor
kind: Kustomization
resources:
- application.yaml
12 changes: 4 additions & 8 deletions aws/nvidia-device-plugin/overlays/application/kustomization.yaml
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
bases:
- ../../base
resources:
- application.yaml
commonLabels:
app.kubernetes.io/name: nvidia-device-plugin
app.kubernetes.io/instance: nvidia-device-plugin-1.0.0-beta
app.kubernetes.io/managed-by: kfctl
app.kubernetes.io/component: nvidia-device-plugin
app.kubernetes.io/part-of: kubeflow
app.kubernetes.io/version: v1.0.0-beta
app.kubernetes.io/name: nvidia-device-plugin
kind: Kustomization
resources:
- application.yaml
16 changes: 6 additions & 10 deletions cert-manager/cert-manager/overlays/application/kustomization.yaml
Original file line number Diff line number Diff line change
@@ -1,15 +1,11 @@
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
bases:
- ../../base
resources:
- application.yaml
configurations:
- params.yaml
commonLabels:
app.kubernetes.io/name: cert-manager
app.kubernetes.io/instance: cert-manager
app.kubernetes.io/managed-by: kfctl
app.kubernetes.io/component: cert-manager
app.kubernetes.io/part-of: kubeflow

app.kubernetes.io/name: cert-manager
configurations:
- params.yaml
kind: Kustomization
resources:
- application.yaml
11 changes: 3 additions & 8 deletions cert-manager/cert-manager/v3/kustomization.yaml
Original file line number Diff line number Diff line change
@@ -1,13 +1,8 @@
# TODO(https://github.com/kubeflow/manifests/issues/1052) clean up the manifests
# after the refactor is done.
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
namespace: cert-manager
commonLabels:
app.kubernetes.io/name: cert-manager
app.kubernetes.io/instance: cert-manager
app.kubernetes.io/managed-by: kfctl
app.kubernetes.io/component: cert-manager
app.kubernetes.io/part-of: kubeflow
app.kubernetes.io/name: cert-manager
kind: Kustomization
namespace: cert-manager
resources:
- ../overlays/application/application.yaml
10 changes: 3 additions & 7 deletions common/centraldashboard/overlays/application/kustomization.yaml
Original file line number Diff line number Diff line change
@@ -1,11 +1,7 @@
apiVersion: kustomize.config.k8s.io/v1beta1
commonLabels:
app.kubernetes.io/component: centraldashboard
app.kubernetes.io/name: centraldashboard
kind: Kustomization
resources:
- application.yaml
commonLabels:
app.kubernetes.io/name: centraldashboard
app.kubernetes.io/instance: centraldashboard-v0.7.0
app.kubernetes.io/managed-by: kfctl
app.kubernetes.io/component: centraldashboard
app.kubernetes.io/part-of: kubeflow
app.kubernetes.io/version: v0.7.0
14 changes: 2 additions & 12 deletions common/centraldashboard/overlays/stacks/kustomization.yaml
Original file line number Diff line number Diff line change
@@ -1,22 +1,12 @@
apiVersion: kustomize.config.k8s.io/v1beta1
commonLabels:
app.kubernetes.io/component: centraldashboard
app.kubernetes.io/instance: centraldashboard-v1.0.0
app.kubernetes.io/managed-by: kfctl
app.kubernetes.io/name: centraldashboard
app.kubernetes.io/part-of: kubeflow
app.kubernetes.io/version: v1.0.0
kind: Kustomization
namespace: kubeflow
patchesStrategicMerge:
- deployment_kf_config.yaml
resources:
- ../../base_v3
# TODO(jlewi): istio and application are really patches
# not "overlays" in that they are expected to be used as mixins.
# Perhaps move this into mixins to make this more obvious.
- ../../overlays/istio
- ../../overlays/application
patchesStrategicMerge:
# Pull in the patch which will configure central dashboard using a kubeflow
# configmap
- deployment_kf_config.yaml

12 changes: 4 additions & 8 deletions common/spartakus/overlays/application/kustomization.yaml
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
bases:
- ../../base
resources:
- application.yaml
commonLabels:
app.kubernetes.io/name: spartakus
app.kubernetes.io/instance: spartakus-v0.7.0
app.kubernetes.io/managed-by: kfctl
app.kubernetes.io/component: spartakus
app.kubernetes.io/part-of: kubeflow
app.kubernetes.io/version: v0.7.0
app.kubernetes.io/name: spartakus
kind: Kustomization
resources:
- application.yaml
12 changes: 4 additions & 8 deletions gcp/basic-auth-ingress/overlays/application/kustomization.yaml
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
bases:
- ../../base
resources:
- application.yaml
commonLabels:
app.kubernetes.io/name: basic-auth-ingress
app.kubernetes.io/instance: basic-auth-ingress-v0.7.0
app.kubernetes.io/managed-by: kfctl
app.kubernetes.io/component: basic-auth-ingress
app.kubernetes.io/part-of: kubeflow
app.kubernetes.io/version: v0.7.0
app.kubernetes.io/name: basic-auth-ingress
kind: Kustomization
resources:
- application.yaml
14 changes: 5 additions & 9 deletions gcp/cloud-endpoints/overlays/application/kustomization.yaml
Original file line number Diff line number Diff line change
@@ -1,14 +1,10 @@
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
namespace: kubeflow
bases:
- ../../base
resources:
- application.yaml
commonLabels:
app.kubernetes.io/name: cloud-endpoints
app.kubernetes.io/instance: cloud-endpoints-v0.7.0
app.kubernetes.io/managed-by: kfctl
app.kubernetes.io/component: cloud-endpoints
app.kubernetes.io/part-of: kubeflow
app.kubernetes.io/version: v0.7.0
app.kubernetes.io/name: cloud-endpoints
kind: Kustomization
namespace: kubeflow
resources:
- application.yaml
12 changes: 4 additions & 8 deletions gcp/gpu-driver/overlays/application/kustomization.yaml
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
bases:
- ../../base
resources:
- application.yaml
commonLabels:
app.kubernetes.io/name: gpu-driver
app.kubernetes.io/instance: gpu-driver-v0.7.0
app.kubernetes.io/managed-by: kfctl
app.kubernetes.io/component: gpu-driver
app.kubernetes.io/part-of: kubeflow
app.kubernetes.io/version: v0.7.0
app.kubernetes.io/name: gpu-driver
kind: Kustomization
resources:
- application.yaml
12 changes: 4 additions & 8 deletions gcp/iap-ingress/overlays/application/kustomization.yaml
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
bases:
- ../../base
resources:
- application.yaml
commonLabels:
app.kubernetes.io/name: iap-ingress
app.kubernetes.io/instance: iap-ingress-v0.7.0
app.kubernetes.io/managed-by: kfctl
app.kubernetes.io/component: iap-ingress
app.kubernetes.io/part-of: kubeflow
app.kubernetes.io/version: v0.7.0
app.kubernetes.io/name: iap-ingress
kind: Kustomization
resources:
- application.yaml
12 changes: 4 additions & 8 deletions gcp/prometheus/overlays/application/kustomization.yaml
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
bases:
- ../../base
resources:
- application.yaml
commonLabels:
app.kubernetes.io/name: prometheus
app.kubernetes.io/instance: prometheus-v0.7.0
app.kubernetes.io/managed-by: kfctl
app.kubernetes.io/component: prometheus
app.kubernetes.io/part-of: kubeflow
app.kubernetes.io/version: v0.7.0
app.kubernetes.io/name: prometheus
kind: Kustomization
resources:
- application.yaml
51 changes: 0 additions & 51 deletions hack/generate_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,55 +80,6 @@ def find_kustomize_dirs(search_dirs):

return changed_dirs

def remove_unmatched_tests(repo_root, package_dirs):
"""Remove any tests that don't map to a kustomization.yaml file.
This ensures tests don't linger if a package is deleted.
"""

# Create a set of all the expected test names
expected_tests = set()

for d in package_dirs:
rpath = os.path.relpath(d, repo_root)
expected_tests.add(generate_test_path(repo_root, rpath))

tests_dir = os.path.join(repo_root, "tests")

possible_empty_dirs = []

# Walk the tests directory
for root, dirs, files in os.walk(tests_dir):
if not files:
possible_empty_dirs.append(root)

for f in files:
if f != TEST_NAME:
continue

full_test = os.path.join(root, f)
if full_test not in expected_tests:
logging.info("Removing unexpected test: %s", full_test)
os.unlink(full_test)
possible_empty_dirs.append(root)

# Prune directories that only contain test_data but no more tests
for d in possible_empty_dirs:
if not os.path.exists(d):
# Might have been a subdirectory of a directory that was deleted.
continue

items = os.listdir(d)

if len(items) > 1:
continue

if len(items) == 1 and items[0] != "test_data":
continue

logging.info("Removing directory: %s", d)
shutil.rmtree(d)

def write_go_test(test_path, package_name, package_dir):
"""Write the go test file.
Expand Down Expand Up @@ -176,8 +127,6 @@ def write_go_test(test_path, package_name, package_dir):
full_search_dirs = [os.path.join(repo_root, s) for s in SEARCH_DIRS]
package_dirs = find_kustomize_dirs(full_search_dirs)

remove_unmatched_tests(repo_root, package_dirs)

changed_dirs = package_dirs

this_dir = os.path.dirname(__file__)
Expand Down
Loading

0 comments on commit 4307f37

Please sign in to comment.