Skip to content

Commit

Permalink
Set dsp-version label on managed resources
Browse files Browse the repository at this point in the history
This change makes it so that new reconciles on dspa owned resources are
labelled with a dsp-version=<dspa-version> label. This is to allow for
the dspa selectively watch (for manual WatchesRawSources) on resources
that are descendents of DSPAs with .spec.dspVersion set to supported
versions.

Signed-off-by: Humair Khan <HumairAK@users.noreply.github.com>
  • Loading branch information
HumairAK committed Oct 18, 2024
1 parent dd9e990 commit 5f23d35
Show file tree
Hide file tree
Showing 33 changed files with 168 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -859,7 +859,7 @@ spec:
status: {}
- deprecated: true
deprecationWarning: datasciencepipelinesapplications.opendatahub.io/v1alpha1 is
deprecated
deprecated.
name: v1alpha1
schema:
openAPIV3Schema:
Expand Down
13 changes: 10 additions & 3 deletions controllers/config/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,13 @@ import (
"k8s.io/apimachinery/pkg/api/resource"
)

const DSPV2VersionString = "v2"
const DSPVersionk8sLabel = "dsp-version"

var SupportedDSPVersions = []string{DSPV2VersionString}

const (
DSPDefaultVersion = "v2"
DSPV2VersionString = DSPDefaultVersion
DefaultImageValue = "MustSetInConfig"
DefaultImageValue = "MustSetInConfig"

CustomCABundleRootMountPath = "/dsp-custom-certs"

Expand Down Expand Up @@ -222,3 +225,7 @@ func GetDefaultDBExtraParams(params DBExtraParams, log logr.Logger) (string, err
}
return string(extraParamsJson), nil
}

func GetSupportedDSPAVersions() []string {
return SupportedDSPVersions
}
46 changes: 32 additions & 14 deletions controllers/dspipeline_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package controllers
import (
"context"
"fmt"

"github.com/opendatahub-io/data-science-pipelines-operator/controllers/dspastatus"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/controller"
Expand Down Expand Up @@ -82,18 +81,25 @@ func (r *DSPAReconciler) Apply(owner mf.Owner, params *DSPAParams, template stri
if err != nil {
return fmt.Errorf("error loading template (%s) yaml: %w", template, err)
}

// Apply the owner injection transformation
tmplManifest, err = tmplManifest.Transform(
mf.InjectOwner(owner),
// Apply dsp-version=<ver> label on all resources managed by this dspo
util.AddLabelTransformer(config.DSPVersionk8sLabel, params.DSPVersion),
util.AddDeploymentPodLabelTransformer(config.DSPVersionk8sLabel, params.DSPVersion),
)
if err != nil {
return err
}

// Apply dsp-version labels to all manifests
tmplManifest, err = tmplManifest.Transform(fns...)
if err != nil {
return err
}

// Apply the manifest
return tmplManifest.Apply()
}

Expand Down Expand Up @@ -188,7 +194,7 @@ func (r *DSPAReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.

defer r.updateStatus(ctx, dspa, dspaStatus, log, req)

if dspa.Spec.DSPVersion != config.DSPV2VersionString {
if !util.DSPAWithSupportedDSPVersion(dspa) {
err1 := fmt.Errorf("unsupported DSP version %s detected. Please manually remove "+
"this DSP resource and re-apply with a supported version field set", dspa.Spec.DSPVersion)
dspaStatus.SetDatabaseNotReady(err1, config.UnsupportedVersion)
Expand Down Expand Up @@ -594,7 +600,6 @@ func (r *DSPAReconciler) SetupWithManager(mgr ctrl.Manager) error {
// Watch for global ca bundle, if one is added to this namespace
// we need to reconcile on all the dspa's in this namespace
// so they may mount this cert in the appropriate containers

WatchesRawSource(source.Kind(mgr.GetCache(), &corev1.ConfigMap{}),
handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, o client.Object) []reconcile.Request {
cm := o.(*corev1.ConfigMap)
Expand All @@ -605,8 +610,6 @@ func (r *DSPAReconciler) SetupWithManager(mgr ctrl.Manager) error {
return nil
}

log.V(1).Info(fmt.Sprintf("Reconcile event triggered by change in event on Global CA Bundle: %s", cm.Name))

var dspaList dspav1.DataSciencePipelinesApplicationList
if err := r.List(ctx, &dspaList, client.InNamespace(thisNamespace)); err != nil {
log.Error(err, "unable to list DSPA's when attempting to handle Global CA Bundle event.")
Expand All @@ -615,11 +618,18 @@ func (r *DSPAReconciler) SetupWithManager(mgr ctrl.Manager) error {

var reconcileRequests []reconcile.Request
for _, dspa := range dspaList.Items {
namespacedName := types.NamespacedName{
Name: dspa.Name,
Namespace: thisNamespace,
// Only update supported DSP versions
if util.DSPAWithSupportedDSPVersion(&dspa) {
namespacedName := types.NamespacedName{
Name: dspa.Name,
Namespace: thisNamespace,
}
reconcileRequests = append(reconcileRequests, reconcile.Request{NamespacedName: namespacedName})
}
reconcileRequests = append(reconcileRequests, reconcile.Request{NamespacedName: namespacedName})
}

if len(reconcileRequests) > 0 {
log.V(1).Info(fmt.Sprintf("Reconcile event triggered by change in event on Global CA Bundle: %s", cm.Name))
}

return reconcileRequests
Expand All @@ -635,6 +645,12 @@ func (r *DSPAReconciler) SetupWithManager(mgr ctrl.Manager) error {
return nil
}

// Silently skip reconcile on this pod if the resource was owned
// by an unsupported dspa
if !util.HasSupportedDSPVersionLabel(pod.Labels) {
return nil
}

dspaName, hasDSPALabel := pod.Labels["dspa"]
if !hasDSPALabel {
msg := fmt.Sprintf("Pod with data-science-pipelines label encountered, but is missing dspa "+
Expand All @@ -659,9 +675,6 @@ func (r *DSPAReconciler) SetupWithManager(mgr ctrl.Manager) error {
if secret.Annotations["openshift.io/owning-component"] != "service-ca" {
return nil
}

log.V(1).Info(fmt.Sprintf("Reconcile event triggered by change on Secret owned by service-ca: %s", secret.Name))

serviceName := secret.Annotations["service.beta.openshift.io/originating-service-name"]

namespacedServiceName := types.NamespacedName{
Expand All @@ -675,17 +688,22 @@ func (r *DSPAReconciler) SetupWithManager(mgr ctrl.Manager) error {
if err != nil {
return nil
}

dspaName, hasDSPALabel := service.Labels["dspa"]
if !hasDSPALabel {
return nil
}

log.V(1).Info(fmt.Sprintf("Reconcile event triggered by [Service: %s] ", serviceName))
// Silently skip reconcile on this ervice if the resource was owned
// by an unsupported DSPA
if !util.HasSupportedDSPVersionLabel(service.Labels) {
return nil
}

namespacedDspaName := types.NamespacedName{
Name: dspaName,
Namespace: secret.Namespace,
}
log.V(1).Info(fmt.Sprintf("Reconcile event triggered by change on Secret: %s owned by service-ca: %s", secret.Name, serviceName))
return []reconcile.Request{{NamespacedName: namespacedDspaName}}
}),
).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ metadata:
name: ds-pipeline-testdsp0
namespace: default
labels:
dsp-version: v2
app: ds-pipeline-testdsp0
component: data-science-pipelines
dspa: testdsp0
Expand All @@ -16,6 +17,7 @@ spec:
template:
metadata:
labels:
dsp-version: v2
app: ds-pipeline-testdsp0
component: data-science-pipelines
dspa: testdsp0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ metadata:
name: ds-pipeline-server-config-testdsp0
namespace: default
labels:
dsp-version: v2
app: ds-pipeline-testdsp0
component: data-science-pipelines
data:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ metadata:
name: mariadb-testdsp0
namespace: default
labels:
dsp-version: v2
app: mariadb-testdsp0
component: data-science-pipelines
dspa: testdsp0
Expand All @@ -19,6 +20,7 @@ spec:
template:
metadata:
labels:
dsp-version: v2
app: mariadb-testdsp0
component: data-science-pipelines
dspa: testdsp0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ metadata:
name: ds-pipeline-persistenceagent-testdsp0
namespace: default
labels:
dsp-version: v2
app: ds-pipeline-persistenceagent-testdsp0
component: data-science-pipelines
dspa: testdsp0
Expand All @@ -18,6 +19,7 @@ spec:
annotations:
cluster-autoscaler.kubernetes.io/safe-to-evict: "true"
labels:
dsp-version: v2
app: ds-pipeline-persistenceagent-testdsp0
component: data-science-pipelines
dspa: testdsp0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ metadata:
name: ds-pipeline-scheduledworkflow-testdsp0
namespace: default
labels:
dsp-version: v2
app: ds-pipeline-scheduledworkflow-testdsp0
component: data-science-pipelines
dspa: testdsp0
Expand All @@ -18,6 +19,7 @@ spec:
annotations:
cluster-autoscaler.kubernetes.io/safe-to-evict: "true"
labels:
dsp-version: v2
app: ds-pipeline-scheduledworkflow-testdsp0
component: data-science-pipelines
dspa: testdsp0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ metadata:
name: ds-pipeline-testdsp2
namespace: default
labels:
dsp-version: v2
app: ds-pipeline-testdsp2
component: data-science-pipelines
dspa: testdsp2
Expand All @@ -16,6 +17,7 @@ spec:
template:
metadata:
labels:
dsp-version: v2
app: ds-pipeline-testdsp2
component: data-science-pipelines
dspa: testdsp2
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ metadata:
name: mariadb-testdsp2
namespace: default
labels:
dsp-version: v2
app: mariadb-testdsp2
component: data-science-pipelines
dspa: testdsp2
Expand All @@ -19,6 +20,7 @@ spec:
template:
metadata:
labels:
dsp-version: v2
app: mariadb-testdsp2
component: data-science-pipelines
dspa: testdsp2
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ metadata:
name: minio-testdsp2
namespace: default
labels:
dsp-version: v2
app: minio-testdsp2
component: data-science-pipelines
dspa: testdsp2
Expand All @@ -18,6 +19,7 @@ spec:
template:
metadata:
labels:
dsp-version: v2
app: minio-testdsp2
component: data-science-pipelines
dspa: testdsp2
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ metadata:
name: ds-pipeline-metadata-envoy-testdsp2
namespace: default
labels:
dsp-version: v2
app: ds-pipeline-metadata-envoy-testdsp2
component: data-science-pipelines
dspa: testdsp2
Expand All @@ -19,6 +20,7 @@ spec:
annotations:
sidecar.istio.io/inject: "false"
labels:
dsp-version: v2
app: ds-pipeline-metadata-envoy-testdsp2
component: data-science-pipelines
dspa: testdsp2
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ metadata:
name: ds-pipeline-metadata-grpc-testdsp2
namespace: default
labels:
dsp-version: v2
app: ds-pipeline-metadata-grpc-testdsp2
component: data-science-pipelines
dspa: testdsp2
Expand All @@ -17,6 +18,7 @@ spec:
template:
metadata:
labels:
dsp-version: v2
app: ds-pipeline-metadata-grpc-testdsp2
component: data-science-pipelines
dspa: testdsp2
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ metadata:
name: ds-pipeline-ui-testdsp2
namespace: default
labels:
dsp-version: v2
app: ds-pipeline-ui-testdsp2
component: data-science-pipelines
dspa: testdsp2
Expand All @@ -18,6 +19,7 @@ spec:
annotations:
cluster-autoscaler.kubernetes.io/safe-to-evict: "true"
labels:
dsp-version: v2
app: ds-pipeline-ui-testdsp2
component: data-science-pipelines
dspa: testdsp2
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ metadata:
name: ds-pipeline-persistenceagent-testdsp2
namespace: default
labels:
dsp-version: v2
app: ds-pipeline-persistenceagent-testdsp2
component: data-science-pipelines
dspa: testdsp2
Expand All @@ -18,6 +19,7 @@ spec:
annotations:
cluster-autoscaler.kubernetes.io/safe-to-evict: "true"
labels:
dsp-version: v2
app: ds-pipeline-persistenceagent-testdsp2
component: data-science-pipelines
dspa: testdsp2
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ metadata:
name: sample-config-testdsp2
namespace: default
labels:
dsp-version: v2
app: ds-pipeline-testdsp2
component: data-science-pipelines
data:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ metadata:
name: ds-pipeline-scheduledworkflow-testdsp2
namespace: default
labels:
dsp-version: v2
app: ds-pipeline-scheduledworkflow-testdsp2
component: data-science-pipelines
dspa: testdsp2
Expand All @@ -18,6 +19,7 @@ spec:
annotations:
cluster-autoscaler.kubernetes.io/safe-to-evict: "true"
labels:
dsp-version: v2
app: ds-pipeline-scheduledworkflow-testdsp2
component: data-science-pipelines
dspa: testdsp2
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ metadata:
name: ds-pipeline-testdsp3
namespace: default
labels:
dsp-version: v2
app: ds-pipeline-testdsp3
component: data-science-pipelines
dspa: testdsp3
Expand All @@ -16,6 +17,7 @@ spec:
template:
metadata:
labels:
dsp-version: v2
app: ds-pipeline-testdsp3
component: data-science-pipelines
dspa: testdsp3
Expand Down
Loading

0 comments on commit 5f23d35

Please sign in to comment.