status: add method to remove an operand version from the VersionGetter

Author: liouk

pkg/operator/status/status_controller.go

@@ -2,22 +2,21 @@ package status
 
 import (
 	"context"
-	"k8s.io/utils/clock"
 	"strings"
 	"time"
 
+	"k8s.io/apimachinery/pkg/api/equality"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	"k8s.io/klog/v2"
+	"k8s.io/utils/clock"
 
 	configv1 "github.com/openshift/api/config/v1"
 	operatorv1 "github.com/openshift/api/operator/v1"
 	configv1client "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1"
 	configv1informers "github.com/openshift/client-go/config/informers/externalversions/config/v1"
 	configv1listers "github.com/openshift/client-go/config/listers/config/v1"
-	"k8s.io/apimachinery/pkg/api/equality"
-	apierrors "k8s.io/apimachinery/pkg/api/errors"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
-
 	configv1helpers "github.com/openshift/library-go/pkg/config/clusteroperator/v1helpers"
 	"github.com/openshift/library-go/pkg/controller/factory"
 	"github.com/openshift/library-go/pkg/operator/events"
@@ -29,6 +28,8 @@ import (
 type VersionGetter interface {
 	// SetVersion is a way to set the version for an operand.  It must be thread-safe
 	SetVersion(operandName, version string)
+	// UnsetVersion removes a version for an operand if it exists; it is a no-op otherwise.  It must be thread-safe
+	UnsetVersion(operandName string)
 	// GetVersion is way to get the versions for all operands.  It must be thread-safe and return an object that doesn't mutate
 	GetVersions() map[string]string
 	// VersionChangedChannel is a channel that will get an item whenever SetVersion has been called

pkg/operator/status/version.go

@@ -35,14 +35,19 @@ func (v *versionGetter) SetVersion(operandName, version string) {
 	defer v.lock.Unlock()
 
 	v.versions[operandName] = version
+	v.notifyChannelsLocked()
+}
 
-	for i := range v.notificationChannels {
-		ch := v.notificationChannels[i]
-		// don't let a slow consumer block the rest
-		go func() {
-			ch <- struct{}{}
-		}()
+func (v *versionGetter) UnsetVersion(operandName string) {
+	v.lock.Lock()
+	defer v.lock.Unlock()
+
+	if _, exists := v.versions[operandName]; !exists {
+		return
 	}
+
+	delete(v.versions, operandName)
+	v.notifyChannelsLocked()
 }
 
 func (v *versionGetter) GetVersions() map[string]string {
@@ -65,6 +70,17 @@ func (v *versionGetter) VersionChangedChannel() <-chan struct{} {
 	return channel
 }
 
+// notifyChannelsLocked must be called under a locked v.lock
+func (v *versionGetter) notifyChannelsLocked() {
+	for i := range v.notificationChannels {
+		ch := v.notificationChannels[i]
+		// don't let a slow consumer block the rest
+		go func() {
+			ch <- struct{}{}
+		}()
+	}
+}
+
 func ImageForOperandFromEnv() string {
 	return os.Getenv(operandImageEnvVarName)
 }

pkg/operator/status/version_test.go

@@ -33,3 +33,45 @@ func TestVersionGetterBasic(t *testing.T) {
 	}
 
 }
+
+func TestVersionGetterUnset(t *testing.T) {
+	versionGetter := NewVersionGetter()
+
+	versionGetter.UnsetVersion("nonexistent")
+	expected := map[string]string{}
+	versions := versionGetter.GetVersions()
+	if !reflect.DeepEqual(expected, versions) {
+		t.Fatalf("Expected %v, got %v", expected, versions)
+	}
+
+	versionGetter.SetVersion("foo", "1.0.0")
+	versionGetter.SetVersion("bar", "2.0.0")
+
+	versions = versionGetter.GetVersions()
+	expected = map[string]string{"foo": "1.0.0", "bar": "2.0.0"}
+	if !reflect.DeepEqual(expected, versions) {
+		t.Fatalf("wanted: %v; got: %v", expected, versions)
+	}
+
+	ch := versionGetter.VersionChangedChannel()
+	versionGetter.UnsetVersion("foo")
+
+	select {
+	case <-ch:
+		// we got notified
+	case <-time.After(5 * time.Second):
+		t.Fatal("expected change notification after UnsetVersion but didn't get any")
+	}
+
+	versions = versionGetter.GetVersions()
+	expected = map[string]string{"bar": "2.0.0"}
+	if !reflect.DeepEqual(expected, versions) {
+		t.Fatalf("Expected %v, got %v", expected, versions)
+	}
+
+	versionGetter.UnsetVersion("nonexistent")
+	versions = versionGetter.GetVersions()
+	if !reflect.DeepEqual(expected, versions) {
+		t.Fatalf("Expected %v, got %v", expected, versions)
+	}
+}