Skip to content

Notebooks 2.0 // Controller // Replace reflect.DeepEqual with equality.Semantic.DeepEqual #69

@thesuperzapper

Description

@thesuperzapper

What's the problem?

We currently use reflect.DeepEqual to do a lot of comparisons in the Notebooks 2.0 controller.

As a result of this, we have to take special care when comparing Kubernetes API objects for a number of reasons:

  1. a nil map is not equal to an empty map
  2. a nil slice is not equal to an empty slice
  3. resource requests need to be carefully compared (as they can be specified in a lot of forms)
  4. time quantities need to be carefully compared (as they can be specified in a lot of forms)

Therefore, we need to use helper method like NormalizePodConfigSpec and other cases to handle this complexity.

What am I proposing?

We can replace this complexity by using Kubernetes built in API object semantic comparison:

import "k8s.io/apimachinery/pkg/api/equality"

...

if equality.Semantic.DeepEqual(object1, object1) {
  return false, nil
}

Next Steps

  1. replace all instances of reflect.DeepEqual with equality.Semantic.DeepEqual
    • NOTE: this should ONLY be changed for comparisons between Kubernetes API objects, or parts of them, not other things we might be comparing.
  2. remove any special methods we are using to compare/mutate the objects before applying the equality check

Metadata

Metadata

Assignees

Labels

area/controllerarea - related to controller componentsarea/v2area - version - kubeflow notebooks v2kind/enhancementkind - new features or changes

Type

No type

Projects

Status

Needs Triage

Relationships

None yet

Development

No branches or pull requests

Issue actions