Skip to content

Commit

Permalink
upgrade: controller-runtime and code change accordingly
Browse files Browse the repository at this point in the history
- the current controller-runtime 0.14.6 does not support new featurs:
- uplift controller-runtime to v0.17.5 which is the last support go 1.21
- update Watches() to match 0.17.5
- webhook in the new 0.16.3 does not use method InjectDecoder InjectClient, updates
- remove MWC since we are not using it for now
- cleanup old go.mod, remove unnecessary replacement
- some formatting for SetupWithManager()
- add missing "get" on console

Signed-off-by: Wen Zhou <wenzhou@redhat.com>
  • Loading branch information
zdtsw committed Aug 19, 2024
1 parent a88cef0 commit 9f7b369
Show file tree
Hide file tree
Showing 24 changed files with 391 additions and 2,218 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,11 @@ kind: Service
metadata:
annotations:
service.beta.openshift.io/inject-cabundle: "true"
service.beta.openshift.io/serving-cert-secret-name: opendatahub-operator-controller-webhook-cert
service.beta.openshift.io/serving-cert-secret-name: opendatahub-operator-webhook-cert
creationTimestamp: null
labels:
app.kubernetes.io/component: webhook
app.kubernetes.io/created-by: opendatahub-operator
app.kubernetes.io/instance: webhook-service
app.kubernetes.io/managed-by: kustomize
app.kubernetes.io/name: service
app.kubernetes.io/part-of: opendatahub-operator
name: opendatahub-operator-webhook-service
spec:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1758,7 +1758,7 @@ spec:
- name: cert
secret:
defaultMode: 420
secretName: opendatahub-operator-controller-webhook-cert
secretName: opendatahub-operator-controller-manager-service-cert
strategy: deployment
installModes:
- supported: false
Expand Down
2 changes: 1 addition & 1 deletion components/kserve/kserve_config_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func (k *Kserve) setDefaultDeploymentMode(ctx context.Context, cli client.Client
Name: KserveConfigMapName,
}, inferenceServiceConfigMap)
if err != nil {
return fmt.Errorf("error getting configmap 'inferenceservice-config'. %w", err)
return fmt.Errorf("error getting configmap %v: %w", KserveConfigMapName, err)
}

// set data.deploy.defaultDeploymentMode to the model specified in the Kserve spec
Expand Down
2 changes: 1 addition & 1 deletion config/crd/kustomization.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ resources:
- bases/features.opendatahub.io_featuretrackers.yaml
#+kubebuilder:scaffold:crdkustomizeresource

patches:
# patches:
# [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix.
# patches here are for enabling the conversion webhook for each CRD
#- patches/webhook_in_dscinitiatlizations.yaml
Expand Down
2 changes: 1 addition & 1 deletion config/default/manager_webhook_patch.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,4 @@ spec:
- name: cert
secret:
defaultMode: 420
secretName: opendatahub-operator-controller-webhook-cert
secretName: opendatahub-operator-webhook-service
1 change: 1 addition & 0 deletions config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1062,6 +1062,7 @@ rules:
- consoles
verbs:
- delete
- get
- list
- patch
- watch
Expand Down
2 changes: 1 addition & 1 deletion config/webhook/kustomization.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@ commonAnnotations:
service.beta.openshift.io/inject-cabundle: "true"

configurations:
- kustomizeconfig.yaml
- kustomizeconfig.yaml
7 changes: 0 additions & 7 deletions config/webhook/kustomizeconfig.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,11 @@ nameReference:
- kind: Service
version: v1
fieldSpecs:
- kind: MutatingWebhookConfiguration
group: admissionregistration.k8s.io
path: webhooks/clientConfig/service/name
- kind: ValidatingWebhookConfiguration
group: admissionregistration.k8s.io
path: webhooks/clientConfig/service/name

namespace:
- kind: MutatingWebhookConfiguration
group: admissionregistration.k8s.io
path: webhooks/clientConfig/service/namespace
create: true
- kind: ValidatingWebhookConfiguration
group: admissionregistration.k8s.io
path: webhooks/clientConfig/service/namespace
Expand Down
7 changes: 1 addition & 6 deletions config/webhook/service.yaml
Original file line number Diff line number Diff line change
@@ -1,18 +1,13 @@

apiVersion: v1
kind: Service
metadata:
labels:
app.kubernetes.io/name: service
app.kubernetes.io/instance: webhook-service
app.kubernetes.io/component: webhook
app.kubernetes.io/created-by: opendatahub-operator
app.kubernetes.io/part-of: opendatahub-operator
app.kubernetes.io/managed-by: kustomize
name: webhook-service
namespace: system
annotations:
service.beta.openshift.io/serving-cert-secret-name: opendatahub-operator-controller-webhook-cert
service.beta.openshift.io/serving-cert-secret-name: opendatahub-operator-webhook-cert
spec:
ports:
- port: 443
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/handler"
"sigs.k8s.io/controller-runtime/pkg/predicate"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
"sigs.k8s.io/controller-runtime/pkg/source"

dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1"
annotation "github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/annotations"
Expand All @@ -37,8 +36,8 @@ func (r *CertConfigmapGeneratorReconciler) SetupWithManager(mgr ctrl.Manager) er
r.Log.Info("Adding controller for Configmap Generation.")
return ctrl.NewControllerManagedBy(mgr).
Named("cert-configmap-generator-controller").
Watches(&source.Kind{Type: &corev1.ConfigMap{}}, handler.EnqueueRequestsFromMapFunc(r.watchTrustedCABundleConfigMapResource), builder.WithPredicates(ConfigMapChangedPredicate)).
Watches(&source.Kind{Type: &corev1.Namespace{}}, handler.EnqueueRequestsFromMapFunc(r.watchNamespaceResource), builder.WithPredicates(NamespaceCreatedPredicate)).
Watches(&corev1.ConfigMap{}, handler.EnqueueRequestsFromMapFunc(r.watchTrustedCABundleConfigMapResource), builder.WithPredicates(ConfigMapChangedPredicate)).
Watches(&corev1.Namespace{}, handler.EnqueueRequestsFromMapFunc(r.watchNamespaceResource), builder.WithPredicates(NamespaceCreatedPredicate)).
Complete(r)
}

Expand Down Expand Up @@ -97,7 +96,7 @@ func (r *CertConfigmapGeneratorReconciler) Reconcile(ctx context.Context, req ct
return ctrl.Result{}, nil
}

func (r *CertConfigmapGeneratorReconciler) watchNamespaceResource(a client.Object) []reconcile.Request {
func (r *CertConfigmapGeneratorReconciler) watchNamespaceResource(_ context.Context, a client.Object) []reconcile.Request {
namespace, isNamespaceObject := a.(*corev1.Namespace)
if !isNamespaceObject {
return nil
Expand All @@ -108,7 +107,7 @@ func (r *CertConfigmapGeneratorReconciler) watchNamespaceResource(a client.Objec
return nil
}

func (r *CertConfigmapGeneratorReconciler) watchTrustedCABundleConfigMapResource(a client.Object) []reconcile.Request {
func (r *CertConfigmapGeneratorReconciler) watchTrustedCABundleConfigMapResource(_ context.Context, a client.Object) []reconcile.Request {
if a.GetName() == trustedcabundle.CAConfigMapName {
r.Log.Info("Cert configmap has been updated, start reconcile")
return []reconcile.Request{{NamespacedName: types.NamespacedName{Name: a.GetName(), Namespace: a.GetNamespace()}}}
Expand Down
168 changes: 102 additions & 66 deletions controllers/datasciencecluster/datasciencecluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/handler"
"sigs.k8s.io/controller-runtime/pkg/predicate"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
"sigs.k8s.io/controller-runtime/pkg/source"

dscv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/datasciencecluster/v1"
dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1"
Expand Down Expand Up @@ -438,77 +437,116 @@ func (r *DataScienceClusterReconciler) SetupWithManager(ctx context.Context, mgr
For(&dscv1.DataScienceCluster{}).
Owns(&corev1.Namespace{}).
Owns(&corev1.Secret{}).
Owns(&corev1.ConfigMap{}, builder.WithPredicates(configMapPredicates)).
Owns(&networkingv1.NetworkPolicy{}, builder.WithPredicates(networkpolicyPredicates)).
Owns(&rbacv1.Role{}, builder.WithPredicates(predicate.Or(predicate.GenerationChangedPredicate{}, modelMeshRolePredicates))).
Owns(&rbacv1.RoleBinding{}, builder.WithPredicates(predicate.Or(predicate.GenerationChangedPredicate{}, modelMeshRBPredicates))).
Owns(&rbacv1.ClusterRole{}, builder.WithPredicates(predicate.Or(predicate.GenerationChangedPredicate{}, modelMeshRolePredicates))).
Owns(&rbacv1.ClusterRoleBinding{}, builder.WithPredicates(predicate.Or(predicate.GenerationChangedPredicate{}, modelMeshRBPredicates))).
Owns(
&corev1.ConfigMap{},
builder.WithPredicates(configMapPredicates),
).
Owns(
&networkingv1.NetworkPolicy{},
builder.WithPredicates(networkpolicyPredicates),
).
Owns(
&rbacv1.Role{},
builder.WithPredicates(predicate.Or(predicate.GenerationChangedPredicate{}, modelMeshRolePredicates))).
Owns(
&rbacv1.RoleBinding{},
builder.WithPredicates(predicate.Or(predicate.GenerationChangedPredicate{}, modelMeshRBPredicates))).
Owns(
&rbacv1.ClusterRole{},
builder.WithPredicates(predicate.Or(predicate.GenerationChangedPredicate{}, modelMeshRolePredicates))).
Owns(
&rbacv1.ClusterRoleBinding{},
builder.WithPredicates(predicate.Or(predicate.GenerationChangedPredicate{}, modelMeshRBPredicates))).
Owns(&appsv1.Deployment{}).
Owns(&corev1.PersistentVolumeClaim{}).
Owns(&corev1.Service{}, builder.WithPredicates(predicate.Or(predicate.GenerationChangedPredicate{}, modelMeshGeneralPredicates))).
Owns(
&corev1.Service{},
builder.WithPredicates(predicate.Or(predicate.GenerationChangedPredicate{}, modelMeshGeneralPredicates))).
Owns(&appsv1.StatefulSet{}).
Owns(&imagev1.ImageStream{}).
Owns(&buildv1.BuildConfig{}).
Owns(&apiregistrationv1.APIService{}).
Owns(&networkingv1.Ingress{}).
Owns(&admissionregistrationv1.MutatingWebhookConfiguration{}).
Owns(&admissionregistrationv1.ValidatingWebhookConfiguration{}, builder.WithPredicates(modelMeshwebhookPredicates)).
Owns(&corev1.ServiceAccount{}, builder.WithPredicates(saPredicates)).
Watches(&source.Kind{Type: &dsciv1.DSCInitialization{}}, handler.EnqueueRequestsFromMapFunc(r.watchDataScienceClusterForDSCI(ctx))).
Watches(&source.Kind{Type: &corev1.ConfigMap{}}, handler.EnqueueRequestsFromMapFunc(r.watchDataScienceClusterResources(ctx)), builder.WithPredicates(configMapPredicates)).
Watches(&source.Kind{Type: &apiextensionsv1.CustomResourceDefinition{}}, handler.EnqueueRequestsFromMapFunc(r.watchDataScienceClusterResources(ctx)),
builder.WithPredicates(argoWorkflowCRDPredicates)).
Watches(&source.Kind{Type: &corev1.Secret{}}, handler.EnqueueRequestsFromMapFunc(r.watchDefaultIngressSecret(ctx)), builder.WithPredicates(defaultIngressCertSecretPredicates)).
Owns(
&admissionregistrationv1.ValidatingWebhookConfiguration{},
builder.WithPredicates(modelMeshwebhookPredicates),
).
Owns(
&corev1.ServiceAccount{},
builder.WithPredicates(saPredicates),
).
Watches(
&dsciv1.DSCInitialization{},
handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, a client.Object) []reconcile.Request {
return r.watchDataScienceClusterForDSCI(ctx, a)
},
)).
Watches(
&corev1.ConfigMap{},
handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, a client.Object) []reconcile.Request {
return r.watchDataScienceClusterResources(ctx, a)
}),
builder.WithPredicates(configMapPredicates),
).
Watches(
&apiextensionsv1.CustomResourceDefinition{},
handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, a client.Object) []reconcile.Request {
return r.watchDataScienceClusterResources(ctx, a)
}),
builder.WithPredicates(argoWorkflowCRDPredicates),
).
Watches(
&corev1.Secret{},
handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, a client.Object) []reconcile.Request {
return r.watchDefaultIngressSecret(ctx, a)
}),
builder.WithPredicates(defaultIngressCertSecretPredicates)).
// this predicates prevents meaningless reconciliations from being triggered
WithEventFilter(predicate.Or(predicate.GenerationChangedPredicate{}, predicate.LabelChangedPredicate{})).
Complete(r)
}

func (r *DataScienceClusterReconciler) watchDataScienceClusterForDSCI(ctx context.Context) func(client.Object) []reconcile.Request {
return func(a client.Object) []reconcile.Request {
requestName, err := r.getRequestName(ctx)
if err != nil {
return nil
}
// When DSCI CR gets created, trigger reconcile function
if a.GetObjectKind().GroupVersionKind().Kind == "DSCInitialization" || a.GetName() == "default-dsci" {
return []reconcile.Request{{
NamespacedName: types.NamespacedName{Name: requestName},
}}
}
func (r *DataScienceClusterReconciler) watchDataScienceClusterForDSCI(ctx context.Context, a client.Object) []reconcile.Request {
requestName, err := r.getRequestName(ctx)
if err != nil {
return nil
}
// When DSCI CR gets created, trigger reconcile function
if a.GetObjectKind().GroupVersionKind().Kind == "DSCInitialization" || a.GetName() == "default-dsci" {
return []reconcile.Request{{
NamespacedName: types.NamespacedName{Name: requestName},
}}
}
return nil
}

func (r *DataScienceClusterReconciler) watchDataScienceClusterResources(ctx context.Context) func(client.Object) []reconcile.Request {
return func(a client.Object) []reconcile.Request {
requestName, err := r.getRequestName(ctx)
if err != nil {
return nil
}
func (r *DataScienceClusterReconciler) watchDataScienceClusterResources(ctx context.Context, a client.Object) []reconcile.Request {
requestName, err := r.getRequestName(ctx)
if err != nil {
return nil
}

if a.GetObjectKind().GroupVersionKind().Kind == "CustomResourceDefinition" || a.GetName() == "ArgoWorkflowCRD" {
return []reconcile.Request{{
NamespacedName: types.NamespacedName{Name: requestName},
}}
}

if a.GetObjectKind().GroupVersionKind().Kind == "CustomResourceDefinition" || a.GetName() == "ArgoWorkflowCRD" {
// Trigger reconcile function when uninstall configmap is created
operatorNs, err := cluster.GetOperatorNamespace()
if err != nil {
return nil
}
if a.GetNamespace() == operatorNs {
cmLabels := a.GetLabels()
if val, ok := cmLabels[upgrade.DeleteConfigMapLabel]; ok && val == "true" {
return []reconcile.Request{{
NamespacedName: types.NamespacedName{Name: requestName},
}}
}

// Trigger reconcile function when uninstall configmap is created
operatorNs, err := cluster.GetOperatorNamespace()
if err != nil {
return nil
}
if a.GetNamespace() == operatorNs {
cmLabels := a.GetLabels()
if val, ok := cmLabels[upgrade.DeleteConfigMapLabel]; ok && val == "true" {
return []reconcile.Request{{
NamespacedName: types.NamespacedName{Name: requestName},
}}
}
}
return nil
}
return nil
}

func (r *DataScienceClusterReconciler) getRequestName(ctx context.Context) (string, error) {
Expand Down Expand Up @@ -543,25 +581,23 @@ var argoWorkflowCRDPredicates = predicate.Funcs{
},
}

func (r *DataScienceClusterReconciler) watchDefaultIngressSecret(ctx context.Context) func(client.Object) []reconcile.Request {
return func(a client.Object) []reconcile.Request {
requestName, err := r.getRequestName(ctx)
if err != nil {
return nil
}
// When ingress secret gets created/deleted, trigger reconcile function
ingressCtrl, err := cluster.FindAvailableIngressController(ctx, r.Client)
if err != nil {
return nil
}
defaultIngressSecretName := cluster.GetDefaultIngressCertSecretName(ingressCtrl)
if a.GetName() == defaultIngressSecretName && a.GetNamespace() == "openshift-ingress" {
return []reconcile.Request{{
NamespacedName: types.NamespacedName{Name: requestName},
}}
}
func (r *DataScienceClusterReconciler) watchDefaultIngressSecret(ctx context.Context, a client.Object) []reconcile.Request {
requestName, err := r.getRequestName(ctx)
if err != nil {
return nil
}
// When ingress secret gets created/deleted, trigger reconcile function
ingressCtrl, err := cluster.FindAvailableIngressController(ctx, r.Client)
if err != nil {
return nil
}
defaultIngressSecretName := cluster.GetDefaultIngressCertSecretName(ingressCtrl)
if a.GetName() == defaultIngressSecretName && a.GetNamespace() == "openshift-ingress" {
return []reconcile.Request{{
NamespacedName: types.NamespacedName{Name: requestName},
}}
}
return nil
}

// defaultIngressCertSecretPredicates filters delete and create events to trigger reconcile when default ingress cert secret is expired
Expand Down
2 changes: 1 addition & 1 deletion controllers/datasciencecluster/kubebuilder_rbac.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ package datasciencecluster

// +kubebuilder:rbac:groups="apiregistration.k8s.io",resources=apiservices,verbs=create;delete;list;watch;update;patch;get

// +kubebuilder:rbac:groups="operator.openshift.io",resources=consoles,verbs=list;watch;patch;delete
// +kubebuilder:rbac:groups="operator.openshift.io",resources=consoles,verbs=get;list;watch;patch;delete
// +kubebuilder:rbac:groups="operator.openshift.io",resources=ingresscontrollers,verbs=get;list;watch;patch;delete

// +kubebuilder:rbac:groups="oauth.openshift.io",resources=oauthclients,verbs=create;delete;list;watch;update;patch;get
Expand Down
Loading

0 comments on commit 9f7b369

Please sign in to comment.