GenericKubernetesResourceMatcher does not detect removals within StatefulSet.spec

[matthew-pintar]

Bug Report

What did you do?

1. Create a StatefulSet in kubernetes which contains more than one container in the spec.template.spec.containers
2. During reconciliation, remove a container so that the desired StatefulSet no longer has it and there are no other changes

What did you expect to see?

The removal of the container from the StatefulSets spec.template.spec.containers is detected as a mismatch and the changes are applied to kubernetes.

What did you see instead? Under which circumstances?

When the GenericKubernetesResourceMatcher.match(R desired, R actualResource, boolean considerMetadata) method is called, it does a resource comparison from desired to actual. It then looks at the operations and if there are any non-add operations, it considers the actual and desired not matching. If there are only add operations (which would be the case since something is being removed from the desired), it would still consider them matching. As a result, it returns the last line return Result.computed(true, desired); so the operator thinks that actual and desired match and does not update kuberenetes.

Environment

Kubernetes cluster type:

docker-desktop and GKE

$ Mention java-operator-sdk version from pom.xml file

io.javaoperatorsdk:operator-framework-junit-5:4.1.0

$ java -version

openjdk version "18" 2022-03-22                                                          
OpenJDK Runtime Environment (build 18+36-2087)
OpenJDK 64-Bit Server VM (build 18+36-2087, mixed mode, sharing)

$ kubectl version

Client Version: v1.26.1
Kustomize Version: v4.5.7
Server Version: v1.24.1

Possible Solution

1. There needs to be a better way to detect when things have changed while ignoring fields that are added by the cluster. Perhaps the operator SDK could add a hash value as a label that is compared instead.

OR

2. There could be a list of fields to ignore during comparison provided to the match method

[csviri]

Hi @matthew-pintar ,

as we discussed maybe in your case it is good enough to use a Matcher with equality, see:
https://github.com/java-operator-sdk/java-operator-sdk/blob/0ea7794f701c270095a37d3eec1b87c355c37a9b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcher.java#L65-L65

But I completely agree that would be nice to support, some ignore list in the matchers. Will take a look on that.

[csviri]

But it is always possible to also provide you custom logic in a dependent resource, just overriding the matcher, in case of Kubernetes Dependent resource, implementing the Matcher interface.

https://github.com/java-operator-sdk/java-operator-sdk/blob/0ea7794f701c270095a37d3eec1b87c355c37a9b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java#L51-L51

[csviri]

@matthew-pintar pls check the related PR, added there basically an ognore list. Also there is an Integration test how to use it. This still ingores only the adds on the listed paths. For example for Service this is needed (see also in the PR), since it is added to specs by k8s:

@KubernetesDependent
public class ServiceDependentResource
    extends CRUDKubernetesDependentResource<Service, ServiceStrictMatcherTestCustomResource> {

// omitted code

 @Override
  public Matcher.Result<Service> match(Service actualResource,
      ServiceStrictMatcherTestCustomResource primary,
      Context<ServiceStrictMatcherTestCustomResource> context) {
    return GenericKubernetesResourceMatcher.match(this, actualResource, primary, context, false,
        false,
        "/spec/ports",
        "/spec/clusterIP",
        "/spec/clusterIPs",
        "/spec/externalTrafficPolicy",
        "/spec/internalTrafficPolicy",
        "/spec/ipFamilies",
        "/spec/ipFamilyPolicy",
        "/spec/sessionAffinity");
  }
  
}

Will be available in the next minor release.

[matthew-pintar]

Awesome! This will work perfectly for me as using a matcher with just equality would cause it to never match in our situation. With the ignore list, it will work for us. Thanks for the quick turn around!