Skip to content

Commit

Permalink
added reconciliation logic notebook controller to add annotation to s…
Browse files Browse the repository at this point in the history
…tatefulset if present in notebook, to exclude image-fields from compare, to keep image-field to new value during reconcile
  • Loading branch information
shalberd committed Oct 24, 2023
1 parent 1ecf41e commit 252f06a
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 5 deletions.
2 changes: 1 addition & 1 deletion components/common/go.mod
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
module github.com/kubeflow/kubeflow/components/common

go 1.17
go 1.19
20 changes: 18 additions & 2 deletions components/common/reconcilehelper/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,11 +100,12 @@ func VirtualService(ctx context.Context, r client.Client, virtualServiceName, na
return nil
}

// Reference: https://github.com/pwittrock/kubebuilder-workshop/blob/master/pkg/util/util.go
// Reference: https://github.com/pwittrock/kubebuilder-workshop/blob/master/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 {
// Also takes into account Openshift image policy admission plug-in on Notebook/StatefulSet metadata annotation to exclude image-field(s) from reconciliation when Pod spec container image field value is done by that plugin
func CopyStatefulSetFields(from, to *appsv1.StatefulSet, isImageChangeTriggerSet bool, imageChangeTriggerReferencedContainerNames []string, log logr.Logger) bool {
requireUpdate := false
for k, v := range to.Labels {
if from.Labels[k] != v {
Expand All @@ -125,6 +126,21 @@ func CopyStatefulSetFields(from, to *appsv1.StatefulSet) bool {
requireUpdate = true
}

// https://stackoverflow.com/questions/47134293/compare-structs-except-one-field-golang
// set container image field of "from" to equal container image field of "to" for all affected container names referenced in image change trigger annotation, if annotation is present
// in that case, to.Spec.Template.Spec.Containers[container].Image is the desired reconcile image value single version of truth, even if it differs from whatever was present in the image field before
// this makes DeepEqual work in this special exclude-image-field from reconciliation/compare scenario, too
if (isImageChangeTriggerSet) {
for containerNameFromAnnotation := range imageChangeTriggerReferencedContainerNames {
for container := range to.Spec.Template.Spec.Containers {
if (to.Spec.Template.Spec.Containers[container].Name == imageChangeTriggerReferencedContainerNames[containerNameFromAnnotation]) {
log.Info("Image Change Trigger Annotation is set", "excluding new container image-field value", to.Spec.Template.Spec.Containers[container].Image, "from DeepEqual and making it the single version of truth by making from/image equal to to/image for container name", to.Spec.Template.Spec.Containers[container].Name)
from.Spec.Template.Spec.Containers[container].Image = to.Spec.Template.Spec.Containers[container].Image
}
}
}
}

if !reflect.DeepEqual(to.Spec.Template.Spec, from.Spec.Template.Spec) {
requireUpdate = true
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"fmt"
"os"
"reflect"
"regexp"
"strings"
"time"

Expand Down Expand Up @@ -52,13 +53,48 @@ const DefaultServingPort = 80
const AnnotationRewriteURI = "notebooks.kubeflow.org/http-rewrite-uri"
const AnnotationHeadersRequestSet = "notebooks.kubeflow.org/http-headers-request-set"
const AnnotationNotebookRestart = "notebooks.opendatahub.io/notebook-restart"
// annotation that makes OpenShift ImagePolicy admission plug-in resolve the image-field from an imagestream name:tag reference
const AnnotationImageChangeTrigger = "image.openshift.io/triggers"

const PrefixEnvVar = "NB_PREFIX"

// The default fsGroup of PodSecurityContext.
// https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.11/#podsecuritycontext-v1-core
const DefaultFSGroup = int64(100)

func getContainerNamesFromAnnotationImageChangeFieldPaths(dataJson string) []string {
annotationImageChangeContent := AnnotationImageChangeContent{}
err := json.Unmarshal([]byte(dataJson), &annotationImageChangeContent)
if err != nil {
fmt.Println("JSON decode error!")
return nil
}

containerNameSlice := make([]string, len(annotationImageChangeContent))

for i := 0; i < len(annotationImageChangeContent); i++ {
re := regexp.MustCompile(`"[^"]+"`)
newStrs := re.FindAllString(annotationImageChangeContent[i].FieldPath, -1)
for _, s := range newStrs {
containerNameSlice[i] = (s[1 : len(s)-1])
}
}

return containerNameSlice
}

func getImageChangeTriggerReferencedContainerNames(meta metav1.ObjectMeta) []string {
if meta.GetAnnotations() == nil {
return nil
}

if valAnnotationImageChangeTrigger, ok := meta.GetAnnotations()[AnnotationImageChangeTrigger]; ok {
return getContainerNamesFromAnnotationImageChangeFieldPaths(valAnnotationImageChangeTrigger)
} else {
return nil
}
}

/*
We generally want to ignore (not requeue) NotFound errors, since we'll get a
reconciliation request once the object exists, and requeuing in the meantime
Expand All @@ -71,6 +107,15 @@ func ignoreNotFound(err error) error {
return err
}

type AnnotationImageChangeContent []struct {
From struct {
Kind string `json:"kind"`
Name string `json:"name"`
Namespace string `json:"namespace"`
} `json:"from"`
FieldPath string `json:"fieldPath"`
}

// NotebookReconciler reconciles a Notebook object
type NotebookReconciler struct {
client.Client
Expand Down Expand Up @@ -159,8 +204,12 @@ func (r *NotebookReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
log.Error(err, "error getting Statefulset")
return ctrl.Result{}, err
}

isImageChangeTriggerSet := metav1.HasAnnotation(instance.ObjectMeta, AnnotationImageChangeTrigger)
imageChangeTriggerReferencedContainerNames := getImageChangeTriggerReferencedContainerNames(instance.ObjectMeta)

// Update the foundStateful object and write the result back if there are any changes
if !justCreated && reconcilehelper.CopyStatefulSetFields(ss, foundStateful) {
if !justCreated && reconcilehelper.CopyStatefulSetFields(ss, foundStateful, isImageChangeTriggerSet, imageChangeTriggerReferencedContainerNames, log) {
log.Info("Updating StatefulSet", "namespace", ss.Namespace, "name", ss.Name)
err = r.Update(ctx, foundStateful)
if err != nil {
Expand Down Expand Up @@ -402,10 +451,19 @@ func generateStatefulSet(instance *v1beta1.Notebook) *appsv1.StatefulSet {
replicas = 0
}

annotations := make(map[string]string)
meta := instance.ObjectMeta

// keep Notebook image change trigger annotation key and value and use it later in StatefulSet metadata
if annotationImageChangeTriggerVal, ok := meta.GetAnnotations()[AnnotationImageChangeTrigger]; ok {
annotations[AnnotationImageChangeTrigger] = annotationImageChangeTriggerVal
}

ss := &appsv1.StatefulSet{
ObjectMeta: metav1.ObjectMeta{
Name: instance.Name,
Namespace: instance.Namespace,
Annotations: annotations,
},
Spec: appsv1.StatefulSetSpec{
Replicas: &replicas,
Expand Down
2 changes: 1 addition & 1 deletion components/notebook-controller/go.mod
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module github.com/kubeflow/kubeflow/components/notebook-controller

go 1.17
go 1.19

// use our version of kubeflow/components/common module for reconcilehelper utility differences related to image-field reconciliation and affected containers exclude
replace (
Expand Down

0 comments on commit 252f06a

Please sign in to comment.