refactor: move common component into each controller (kubeflow#702)

yehuditkerido · Yehudit Kerido · web-flow · commit 8c44fd768b61 · 2025-11-06T20:36:30.000Z
- Delete common component (OWNERS, go.mod, reconcilehelper/)
- Copy reconcilehelper/util.go to both notebook-controller and tensorboard-controller
- Update imports in both controllers to use local reconcilehelper
- Remove common dependency from go.mod files
- Update Dockerfiles to remove common component references

Signed-off-by: Yehudit Kerido &lt;ykerido@ykerido-thinkpadp1gen7.raanaii.csb&gt;
Co-authored-by: Yehudit Kerido &lt;ykerido@ykerido-thinkpadp1gen7.raanaii.csb&gt;
diff --git a/components/common/OWNERS b/components/common/OWNERS
diff --git a/components/common/go.mod b/components/common/go.mod
diff --git a/components/notebook-controller/Dockerfile b/components/notebook-controller/Dockerfile
@@ -4,16 +4,13 @@
 #
 # ${PATH_TO_KUBEFLOW/KUBEFLOW repo}/components
 #
-# This is necessary because the Jupyter controller now depends on
-# components/common
 ARG GOLANG_VERSION=1.17
 FROM golang:${GOLANG_VERSION} as builder
 
 WORKDIR /workspace
 
 # Copy the Go Modules manifests
 COPY notebook-controller /workspace/notebook-controller
-COPY common /workspace/common
 
 # cache deps before building and copying source so that we don't need to re-download as much
 # and so that source changes don't invalidate our downloaded layer
diff --git a/components/notebook-controller/controllers/notebook_controller.go b/components/notebook-controller/controllers/notebook_controller.go
@@ -25,7 +25,7 @@ import (
 	"time"
 
 	"github.com/go-logr/logr"
-	reconcilehelper "github.com/kubeflow/kubeflow/components/common/reconcilehelper"
+	reconcilehelper "github.com/kubeflow/kubeflow/components/notebook-controller/reconcilehelper"
 	"github.com/kubeflow/kubeflow/components/notebook-controller/api/v1beta1"
 	"github.com/kubeflow/kubeflow/components/notebook-controller/pkg/metrics"
 	appsv1 "k8s.io/api/apps/v1"
diff --git a/components/notebook-controller/go.mod b/components/notebook-controller/go.mod
@@ -4,7 +4,6 @@ go 1.17
 
 require (
 	github.com/go-logr/logr v1.2.0
-	github.com/kubeflow/kubeflow/components/common v0.0.0-20220218084159-4ad0158e955e
 	github.com/onsi/ginkgo v1.16.5
 	github.com/onsi/gomega v1.17.0
 	github.com/prometheus/client_golang v1.11.1
diff --git a/components/notebook-controller/go.sum b/components/notebook-controller/go.sum
@@ -314,8 +314,6 @@ github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
 github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
 github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
 github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
-github.com/kubeflow/kubeflow/components/common v0.0.0-20220218084159-4ad0158e955e h1:Ud6a+mkZ5pj+QqZnsIPPBk1WrABz1wYcd1n9CYjMMBA=
-github.com/kubeflow/kubeflow/components/common v0.0.0-20220218084159-4ad0158e955e/go.mod h1:uj1ImonZ+hHqWKfzZGWjuxl1uODjubBNrg4W8c38/ts=
 github.com/magiconair/properties v1.8.1/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ=
 github.com/magiconair/properties v1.8.5/go.mod h1:y3VJvCyxH9uVvJTWEGAELF3aiYNyPKd5NZ3oSwXrF60=
 github.com/mailru/easyjson v0.0.0-20190614124828-94de47d64c63/go.mod h1:C1wdFJiN94OJF2b5HbByQZoLdCWB1Yqtg26g4irojpc=
diff --git a/components/notebook-controller/reconcilehelper/util.go b/components/notebook-controller/reconcilehelper/util.go
diff --git a/components/notebook-controller/third_party/dep.txt b/components/notebook-controller/third_party/dep.txt
@@ -147,7 +147,6 @@ github.com/kr/logfmt
 github.com/kr/pretty
 github.com/kr/pty
 github.com/kr/text
-github.com/kubeflow/kubeflow/components/common
 github.com/magiconair/properties
 github.com/mailru/easyjson
 github.com/mattn/go-colorable
diff --git a/components/tensorboard-controller/Dockerfile b/components/tensorboard-controller/Dockerfile
@@ -3,14 +3,11 @@
 #
 # ${PATH_TO_KUBEFLOW/KUBEFLOW repo}/components
 #
-# This is necessary because the Jupyter controller now depends on
-# components/common
 FROM golang:1.17 as builder
 
 WORKDIR /workspace
 # Copy the Go Modules manifests
 COPY tensorboard-controller /workspace/tensorboard-controller
-COPY common /workspace/common
 # cache deps before building and copying source so that we don't need to re-download as much
 # and so that source changes don't invalidate our downloaded layer
 RUN cd /workspace/tensorboard-controller && go mod download
diff --git a/components/tensorboard-controller/controllers/tensorboard_controller.go b/components/tensorboard-controller/controllers/tensorboard_controller.go
@@ -37,7 +37,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
-	reconcilehelper "github.com/kubeflow/kubeflow/components/common/reconcilehelper"
+	reconcilehelper "github.com/kubeflow/kubeflow/components/tensorboard-controller/reconcilehelper"
 	tensorboardv1alpha1 "github.com/kubeflow/kubeflow/components/tensorboard-controller/api/v1alpha1"
 )
 
diff --git a/components/tensorboard-controller/go.mod b/components/tensorboard-controller/go.mod
@@ -5,7 +5,6 @@ go 1.17
 require (
 	github.com/go-logr/logr v1.2.0
 	github.com/gogo/protobuf v1.3.2
-	github.com/kubeflow/kubeflow/components/common v0.0.0-20220309223711-d224549f11b6
 	github.com/onsi/ginkgo v1.16.5
 	github.com/onsi/gomega v1.17.0
 	k8s.io/api v0.23.0
diff --git a/components/tensorboard-controller/go.sum b/components/tensorboard-controller/go.sum
@@ -314,8 +314,6 @@ github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
 github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
 github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
 github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
-github.com/kubeflow/kubeflow/components/common v0.0.0-20220309223711-d224549f11b6 h1:Wj1vVIDqUdjXdYhg4UMFnOt4V+pazhbw9kpXMLBEeUU=
-github.com/kubeflow/kubeflow/components/common v0.0.0-20220309223711-d224549f11b6/go.mod h1:uj1ImonZ+hHqWKfzZGWjuxl1uODjubBNrg4W8c38/ts=
 github.com/magiconair/properties v1.8.1/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ=
 github.com/magiconair/properties v1.8.5/go.mod h1:y3VJvCyxH9uVvJTWEGAELF3aiYNyPKd5NZ3oSwXrF60=
 github.com/mailru/easyjson v0.0.0-20190614124828-94de47d64c63/go.mod h1:C1wdFJiN94OJF2b5HbByQZoLdCWB1Yqtg26g4irojpc=
diff --git a/components/tensorboard-controller/reconcilehelper/util.go b/components/tensorboard-controller/reconcilehelper/util.go
@@ -0,0 +1,219 @@
+package reconcile
+
+import (
+	"context"
+	"reflect"
+
+	"github.com/go-logr/logr"
+
+	appsv1 "k8s.io/api/apps/v1"
+	corev1 "k8s.io/api/core/v1"
+	apierrs "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/types"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+// Deployment reconciles a k8s deployment object.
+func Deployment(ctx context.Context, r client.Client, deployment *appsv1.Deployment, log logr.Logger) error {
+	foundDeployment := &appsv1.Deployment{}
+	justCreated := false
+	if err := r.Get(ctx, types.NamespacedName{Name: deployment.Name, Namespace: deployment.Namespace}, foundDeployment); err != nil {
+		if apierrs.IsNotFound(err) {
+			log.Info("Creating Deployment", "namespace", deployment.Namespace, "name", deployment.Name)
+			if err := r.Create(ctx, deployment); err != nil {
+				log.Error(err, "unable to create deployment")
+				return err
+			}
+			justCreated = true
+		} else {
+			log.Error(err, "error getting deployment")
+			return err
+		}
+	}
+	if !justCreated && CopyDeploymentSetFields(deployment, foundDeployment) {
+		log.Info("Updating Deployment", "namespace", deployment.Namespace, "name", deployment.Name)
+		if err := r.Update(ctx, foundDeployment); err != nil {
+			log.Error(err, "unable to update deployment")
+			return err
+		}
+	}
+
+	return nil
+}
+
+// Service reconciles a k8s service object.
+func Service(ctx context.Context, r client.Client, service *corev1.Service, log logr.Logger) error {
+	foundService := &corev1.Service{}
+	justCreated := false
+	if err := r.Get(ctx, types.NamespacedName{Name: service.Name, Namespace: service.Namespace}, foundService); err != nil {
+		if apierrs.IsNotFound(err) {
+			log.Info("Creating Service", "namespace", service.Namespace, "name", service.Name)
+			if err = r.Create(ctx, service); err != nil {
+				log.Error(err, "unable to create service")
+				return err
+			}
+			justCreated = true
+		} else {
+			log.Error(err, "error getting service")
+			return err
+		}
+	}
+	if !justCreated && CopyServiceFields(service, foundService) {
+		log.Info("Updating Service\n", "namespace", service.Namespace, "name", service.Name)
+		if err := r.Update(ctx, foundService); err != nil {
+			log.Error(err, "unable to update Service")
+			return err
+		}
+	}
+
+	return nil
+}
+
+// VirtualService reconciles an Istio virtual service object.
+func VirtualService(ctx context.Context, r client.Client, virtualServiceName, namespace string, virtualservice *unstructured.Unstructured, log logr.Logger) error {
+	foundVirtualService := &unstructured.Unstructured{}
+	foundVirtualService.SetAPIVersion("networking.istio.io/v1alpha3")
+	foundVirtualService.SetKind("VirtualService")
+	justCreated := false
+	if err := r.Get(ctx, types.NamespacedName{Name: virtualServiceName, Namespace: namespace}, foundVirtualService); err != nil {
+		if apierrs.IsNotFound(err) {
+			log.Info("Creating virtual service", "namespace", namespace, "name", virtualServiceName)
+			if err := r.Create(ctx, virtualservice); err != nil {
+				log.Error(err, "unable to create virtual service")
+				return err
+			}
+			justCreated = true
+		} else {
+			log.Error(err, "error getting virtual service")
+			return err
+		}
+	}
+	if !justCreated && CopyVirtualService(virtualservice, foundVirtualService) {
+		log.Info("Updating virtual service", "namespace", namespace, "name", virtualServiceName)
+		if err := r.Update(ctx, foundVirtualService); err != nil {
+			log.Error(err, "unable to update virtual service")
+			return err
+		}
+	}
+
+	return nil
+}
+
+// Reference: https://github.com/pwittrock/kubebuilder-workshop/blob/master/pkg/util/util.go
+
+// CopyStatefulSetFields copies the owned fields from one StatefulSet to another
+// Returns true if the fields copied from don't match to.
+func CopyStatefulSetFields(from, to *appsv1.StatefulSet) bool {
+	requireUpdate := false
+	for k, v := range to.Labels {
+		if from.Labels[k] != v {
+			requireUpdate = true
+		}
+	}
+	to.Labels = from.Labels
+
+	for k, v := range to.Annotations {
+		if from.Annotations[k] != v {
+			requireUpdate = true
+		}
+	}
+	to.Annotations = from.Annotations
+
+	if *from.Spec.Replicas != *to.Spec.Replicas {
+		*to.Spec.Replicas = *from.Spec.Replicas
+		requireUpdate = true
+	}
+
+	if !reflect.DeepEqual(to.Spec.Template.Spec, from.Spec.Template.Spec) {
+		requireUpdate = true
+	}
+	to.Spec.Template.Spec = from.Spec.Template.Spec
+
+	return requireUpdate
+}
+
+func CopyDeploymentSetFields(from, to *appsv1.Deployment) bool {
+	requireUpdate := false
+	for k, v := range to.Labels {
+		if from.Labels[k] != v {
+			requireUpdate = true
+		}
+	}
+	to.Labels = from.Labels
+
+	for k, v := range to.Annotations {
+		if from.Annotations[k] != v {
+			requireUpdate = true
+		}
+	}
+	to.Annotations = from.Annotations
+
+	if from.Spec.Replicas != to.Spec.Replicas {
+		to.Spec.Replicas = from.Spec.Replicas
+		requireUpdate = true
+	}
+
+	if !reflect.DeepEqual(to.Spec.Template.Spec, from.Spec.Template.Spec) {
+		requireUpdate = true
+	}
+	to.Spec.Template.Spec = from.Spec.Template.Spec
+
+	return requireUpdate
+}
+
+// CopyServiceFields copies the owned fields from one Service to another
+func CopyServiceFields(from, to *corev1.Service) bool {
+	requireUpdate := false
+	for k, v := range to.Labels {
+		if from.Labels[k] != v {
+			requireUpdate = true
+		}
+	}
+	to.Labels = from.Labels
+
+	for k, v := range to.Annotations {
+		if from.Annotations[k] != v {
+			requireUpdate = true
+		}
+	}
+	to.Annotations = from.Annotations
+
+	// Don't copy the entire Spec, because we can't overwrite the clusterIp field
+
+	if !reflect.DeepEqual(to.Spec.Selector, from.Spec.Selector) {
+		requireUpdate = true
+	}
+	to.Spec.Selector = from.Spec.Selector
+
+	if !reflect.DeepEqual(to.Spec.Ports, from.Spec.Ports) {
+		requireUpdate = true
+	}
+	to.Spec.Ports = from.Spec.Ports
+
+	return requireUpdate
+}
+
+// Copy configuration related fields to another instance and returns true if there
+// is a diff and thus needs to update.
+func CopyVirtualService(from, to *unstructured.Unstructured) bool {
+	fromSpec, found, err := unstructured.NestedMap(from.Object, "spec")
+	if !found {
+		return false
+	}
+	if err != nil {
+		return false
+	}
+
+	toSpec, found, err := unstructured.NestedMap(to.Object, "spec")
+	if !found || err != nil {
+		unstructured.SetNestedMap(to.Object, fromSpec, "spec")
+		return true
+	}
+
+	requiresUpdate := !reflect.DeepEqual(fromSpec, toSpec)
+	if requiresUpdate {
+		unstructured.SetNestedMap(to.Object, fromSpec, "spec")
+	}
+	return requiresUpdate
+}

Original file line number	Diff line number	Diff line change
`@@ -4,16 +4,13 @@`
`4`	`4`	`#`
`5`	`5`	`# ${PATH_TO_KUBEFLOW/KUBEFLOW repo}/components`
`6`	`6`	`#`
`7`		`-# This is necessary because the Jupyter controller now depends on`
`8`		`-# components/common`
`9`	`7`	`ARG GOLANG_VERSION=1.17`
`10`	`8`	`FROM golang:${GOLANG_VERSION} as builder`
`11`	`9`
`12`	`10`	`WORKDIR /workspace`
`13`	`11`
`14`	`12`	`# Copy the Go Modules manifests`
`15`	`13`	`COPY notebook-controller /workspace/notebook-controller`
`16`		`-COPY common /workspace/common`
`17`	`14`
`18`	`15`	`# cache deps before building and copying source so that we don't need to re-download as much`
`19`	`16`	`# and so that source changes don't invalidate our downloaded layer`
-Original file line number
+Diff line change
 require (
 	github.com/go-logr/logr v1.2.0
 -	github.com/kubeflow/kubeflow/components/common v0.0.0-20220218084159-4ad0158e955e
 	github.com/onsi/ginkgo v1.16.5
 	github.com/onsi/gomega v1.17.0
 	github.com/prometheus/client_golang v1.11.1
Original file line number	Diff line number	Diff line change
`@@ -3,14 +3,11 @@`
`3`	`3`	`#`
`4`	`4`	`# ${PATH_TO_KUBEFLOW/KUBEFLOW repo}/components`
`5`	`5`	`#`
`6`		`-# This is necessary because the Jupyter controller now depends on`
`7`		`-# components/common`
`8`	`6`	`FROM golang:1.17 as builder`
`9`	`7`
`10`	`8`	`WORKDIR /workspace`
`11`	`9`	`# Copy the Go Modules manifests`
`12`	`10`	`COPY tensorboard-controller /workspace/tensorboard-controller`
`13`		`-COPY common /workspace/common`
`14`	`11`	`# cache deps before building and copying source so that we don't need to re-download as much`
`15`	`12`	`# and so that source changes don't invalidate our downloaded layer`
`16`	`13`	`RUN cd /workspace/tensorboard-controller && go mod download`
Original file line number	Diff line number	Diff line change
`@@ -37,7 +37,7 @@ import (`
`37`	`37`	`"sigs.k8s.io/controller-runtime/pkg/client"`
`38`	`38`	`"sigs.k8s.io/controller-runtime/pkg/reconcile"`
`39`	`39`
`40`		`- reconcilehelper "github.com/kubeflow/kubeflow/components/common/reconcilehelper"`
	`40`	`+ reconcilehelper "github.com/kubeflow/kubeflow/components/tensorboard-controller/reconcilehelper"`
`41`	`41`	`tensorboardv1alpha1 "github.com/kubeflow/kubeflow/components/tensorboard-controller/api/v1alpha1"`
`42`	`42`	`)`
`43`	`43`