pkg/clusterconditions/promql: Warm cache with 1s delay

cmd/main.go

@@ -5,9 +5,6 @@ import (
 
 	"github.com/spf13/cobra"
 	"k8s.io/klog/v2"
-
-	_ "github.com/openshift/cluster-version-operator/pkg/clusterconditions/always"
-	_ "github.com/openshift/cluster-version-operator/pkg/clusterconditions/promql"
 )
 
 var (

install/0000_00_cluster-version-operator_01_admingate_configmap.yaml

@@ -1,5 +1,8 @@
 apiVersion: v1
 kind: ConfigMap
+data:
+  ack-4.12-kube-1.26-api-removals-in-4.13: |-
+      Kubernetes 1.26 and therefore OpenShift 4.13 remove several APIs which require admin consideration. Please see the knowledge article https://access.redhat.com/articles/6958394 for details and instructions.
 metadata:
   name: admin-gates
   namespace: openshift-config-managed

install/0000_00_cluster-version-operator_03_deployment.yaml

@@ -66,7 +66,9 @@ spec:
                 fieldPath: spec.nodeName
           - name: CLUSTER_PROFILE
             value: {{ .ClusterProfile }}
-      dnsPolicy: ClusterFirstWithHostNet
+      # this pod is hostNetwork and uses the internal LB DNS name when possible, which the kubelet also uses.
+      # this dnsPolicy allows us to use the same dnsConfig as the kubelet, without access to read it ourselves.
+      dnsPolicy: Default
       hostNetwork: true
       nodeSelector:
         node-role.kubernetes.io/master: ""

lib/resourcemerge/core.go

@@ -112,13 +112,6 @@ func ensureContainer(modified *bool, existing *corev1.Container, required corev1
 
 func ensureEnvVar(modified *bool, existing *[]corev1.EnvVar, required []corev1.EnvVar) {
 	for envidx := range required {
-		// Currently only CVO deployment uses this variable to inject internal LB host.
-		// This may result in an IP address being returned by API so assuming the
-		// returned value is correct.
-		if required[envidx].Name == "KUBERNETES_SERVICE_HOST" {
-			ensureEnvVarKubeService(*existing, &required[envidx])
-		}
-
 		if required[envidx].ValueFrom != nil {
 			ensureEnvVarSourceFieldRefDefault(required[envidx].ValueFrom.FieldRef)
 		}
@@ -129,14 +122,6 @@ func ensureEnvVar(modified *bool, existing *[]corev1.EnvVar, required []corev1.E
 	}
 }
 
-func ensureEnvVarKubeService(existing []corev1.EnvVar, required *corev1.EnvVar) {
-	for envidx := range existing {
-		if existing[envidx].Name == required.Name {
-			required.Value = existing[envidx].Value
-		}
-	}
-}
-
 func ensureEnvVarSourceFieldRefDefault(required *corev1.ObjectFieldSelector) {
 	if required != nil && required.APIVersion == "" {
 		required.APIVersion = "v1"

lib/resourcemerge/core_test.go

@@ -1640,6 +1640,26 @@ func TestEnsureEnvVar(t *testing.T) {
 			},
 			expectedModified: false,
 		},
+		{
+			name: "CVO can inject LB into ENVVAR",
+			existing: []corev1.EnvVar{
+				{Name: "ENVVAR", Value: "127.0.0.1"},
+			},
+			input: []corev1.EnvVar{
+				{Name: "ENVVAR", Value: "api-int.ci-ln-vqs15yk-72292.gcp-2.ci.openshift.org"},
+			},
+			expectedModified: true,
+		},
+		{
+			name: "CVO can inject LB into KUBERNETES_SERVICE_HOST",
+			existing: []corev1.EnvVar{
+				{Name: "KUBERNETES_SERVICE_HOST", Value: "127.0.0.1"},
+			},
+			input: []corev1.EnvVar{
+				{Name: "KUBERNETES_SERVICE_HOST", Value: "api-int.ci-ln-vqs15yk-72292.gcp-2.ci.openshift.org"},
+			},
+			expectedModified: true,
+		},
 	}
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {

pkg/cincinnati/cincinnati.go

@@ -30,13 +30,18 @@ const (
 // Client is a Cincinnati client which can be used to fetch update graphs from
 // an upstream Cincinnati stack.
 type Client struct {
-	id        uuid.UUID
-	transport *http.Transport
+	id                uuid.UUID
+	transport         *http.Transport
+	conditionRegistry clusterconditions.ConditionRegistry
 }
 
 // NewClient creates a new Cincinnati client with the given client identifier.
-func NewClient(id uuid.UUID, transport *http.Transport) Client {
-	return Client{id: id, transport: transport}
+func NewClient(id uuid.UUID, transport *http.Transport, conditionRegistry clusterconditions.ConditionRegistry) Client {
+	return Client{
+		id:                id,
+		transport:         transport,
+		conditionRegistry: conditionRegistry,
+	}
 }
 
 // Error is returned when are unable to get updates.
@@ -216,7 +221,7 @@ func (c Client) GetUpdates(ctx context.Context, uri *url.URL, arch string, chann
 
 	for i := len(conditionalUpdates) - 1; i >= 0; i-- {
 		for j, risk := range conditionalUpdates[i].Risks {
-			conditionalUpdates[i].Risks[j].MatchingRules, err = clusterconditions.PruneInvalid(ctx, risk.MatchingRules)
+			conditionalUpdates[i].Risks[j].MatchingRules, err = c.conditionRegistry.PruneInvalid(ctx, risk.MatchingRules)
 			if len(conditionalUpdates[i].Risks[j].MatchingRules) == 0 {
 				klog.Warningf("Conditional update to %s, risk %q, has empty pruned matchingRules; dropping this target to avoid rejections when pushing to the Kubernetes API server. Pruning results: %s", conditionalUpdates[i].Release.Version, risk.Name, err)
 				conditionalUpdates = append(conditionalUpdates[:i], conditionalUpdates[i+1:]...)

pkg/cincinnati/cincinnati_test.go

@@ -10,11 +10,11 @@ import (
 	"reflect"
 	"testing"
 
+	"github.com/openshift/cluster-version-operator/pkg/clusterconditions/standard"
+
 	"github.com/blang/semver/v4"
 	"github.com/google/uuid"
 	configv1 "github.com/openshift/api/config/v1"
-	_ "github.com/openshift/cluster-version-operator/pkg/clusterconditions/always"
-	_ "github.com/openshift/cluster-version-operator/pkg/clusterconditions/promql"
 	_ "k8s.io/klog/v2" // integration tests set glog flags.
 )
 
@@ -604,7 +604,7 @@ func TestGetUpdates(t *testing.T) {
 			ts := httptest.NewServer(http.HandlerFunc(handler))
 			defer ts.Close()
 
-			c := NewClient(clientID, nil)
+			c := NewClient(clientID, nil, standard.NewConditionRegistry(nil))
 
 			uri, err := url.Parse(ts.URL)
 			if err != nil {

pkg/clusterconditions/always/always.go

@@ -8,14 +8,11 @@ import (
 	"errors"
 
 	configv1 "github.com/openshift/api/config/v1"
-	"github.com/openshift/cluster-version-operator/pkg/clusterconditions"
 )
 
 // Always implements a cluster condition that always matches.
 type Always struct{}
 
-var always = &Always{}
-
 // Valid returns an error if the condition contains any properties
 // besides 'type'.
 func (a *Always) Valid(ctx context.Context, condition *configv1.ClusterCondition) error {
@@ -30,7 +27,3 @@ func (a *Always) Valid(ctx context.Context, condition *configv1.ClusterCondition
 func (a *Always) Match(ctx context.Context, condition *configv1.ClusterCondition) (bool, error) {
 	return true, nil
 }
-
-func init() {
-	clusterconditions.Register("Always", always)
-}

pkg/clusterconditions/cache/cache.go

@@ -117,6 +117,8 @@ func (c *Cache) Match(ctx context.Context, condition *configv1.ClusterCondition)
 			detail = fmt.Sprintf(" (last evaluated on %s)", thiefResult.When)
 		}
 		klog.V(2).Infof("%s is the most stale cached cluster-condition match entry, but it is too fresh%s.  However, we don't have a cached evaluation for %s, so attempt to evaluate that now.", thiefKey, detail, key)
+		thiefKey = key
+		targetCondition = condition
 	}
 
 	// if we ended up stealing this Match call, log that, to make contention more clear

pkg/clusterconditions/clusterconditions.go

@@ -25,28 +25,51 @@ type Condition interface {
 	Match(ctx context.Context, condition *configv1.ClusterCondition) (bool, error)
 }
 
-// Registry is a registry of implemented condition types.
-var Registry map[string]Condition
+type ConditionRegistry interface {
+	// Register registers a condition type, and panics on any name collisions.
+	Register(conditionType string, condition Condition)
+
+	// PruneInvalid returns a new slice with recognized, valid conditions.
+	// The error complains about any unrecognized or invalid conditions.
+	PruneInvalid(ctx context.Context, matchingRules []configv1.ClusterCondition) ([]configv1.ClusterCondition, error)
+
+	// Match returns whether the cluster matches the given rules (true),
+	// does not match (false), or the rules fail to evaluate (error).
+	Match(ctx context.Context, matchingRules []configv1.ClusterCondition) (bool, error)
+}
+
+type conditionRegistry struct {
+	// registry is a registry of implemented condition types.
+	registry map[string]Condition
+}
+
+func NewConditionRegistry() ConditionRegistry {
+	ret := &conditionRegistry{
+		registry: map[string]Condition{},
+	}
+
+	return ret
+}
 
 // Register registers a condition type, and panics on any name collisions.
-func Register(conditionType string, condition Condition) {
-	if Registry == nil {
-		Registry = make(map[string]Condition, 1)
+func (r *conditionRegistry) Register(conditionType string, condition Condition) {
+	if r.registry == nil {
+		r.registry = make(map[string]Condition, 1)
 	}
-	if existing, ok := Registry[conditionType]; ok && condition != existing {
+	if existing, ok := r.registry[conditionType]; ok && condition != existing {
 		panic(fmt.Sprintf("cluster condition %q already registered", conditionType))
 	}
-	Registry[conditionType] = condition
+	r.registry[conditionType] = condition
 }
 
 // PruneInvalid returns a new slice with recognized, valid conditions.
 // The error complains about any unrecognized or invalid conditions.
-func PruneInvalid(ctx context.Context, matchingRules []configv1.ClusterCondition) ([]configv1.ClusterCondition, error) {
+func (r *conditionRegistry) PruneInvalid(ctx context.Context, matchingRules []configv1.ClusterCondition) ([]configv1.ClusterCondition, error) {
 	var valid []configv1.ClusterCondition
 	var errs []error
 
 	for _, config := range matchingRules {
-		condition, ok := Registry[config.Type]
+		condition, ok := r.registry[config.Type]
 		if !ok {
 			errs = append(errs, fmt.Errorf("Skipping unrecognized cluster condition type %q", config.Type))
 			continue
@@ -63,11 +86,11 @@ func PruneInvalid(ctx context.Context, matchingRules []configv1.ClusterCondition
 
 // Match returns whether the cluster matches the given rules (true),
 // does not match (false), or the rules fail to evaluate (error).
-func Match(ctx context.Context, matchingRules []configv1.ClusterCondition) (bool, error) {
+func (r *conditionRegistry) Match(ctx context.Context, matchingRules []configv1.ClusterCondition) (bool, error) {
 	var errs []error
 
 	for _, config := range matchingRules {
-		condition, ok := Registry[config.Type]
+		condition, ok := r.registry[config.Type]
 		if !ok {
 			klog.V(2).Infof("Skipping unrecognized cluster condition type %q", config.Type)
 			continue

pkg/clusterconditions/clusterconditions_test.go

@@ -8,10 +8,7 @@ import (
 	"testing"
 
 	configv1 "github.com/openshift/api/config/v1"
-
-	"github.com/openshift/cluster-version-operator/pkg/clusterconditions"
-	_ "github.com/openshift/cluster-version-operator/pkg/clusterconditions/always"
-	_ "github.com/openshift/cluster-version-operator/pkg/clusterconditions/promql"
+	"github.com/openshift/cluster-version-operator/pkg/clusterconditions/standard"
 )
 
 // Error implements a cluster condition that always errors.
@@ -33,6 +30,7 @@ func (e *Error) Match(ctx context.Context, condition *configv1.ClusterCondition)
 
 func TestPruneInvalid(t *testing.T) {
 	ctx := context.Background()
+	registry := standard.NewConditionRegistry(nil)
 
 	for _, testCase := range []struct {
 		name          string
@@ -100,7 +98,7 @@ func TestPruneInvalid(t *testing.T) {
 		},
 	} {
 		t.Run(testCase.name, func(t *testing.T) {
-			valid, err := clusterconditions.PruneInvalid(ctx, testCase.conditions)
+			valid, err := registry.PruneInvalid(ctx, testCase.conditions)
 			if !reflect.DeepEqual(valid, testCase.expectedValid) {
 				t.Errorf("got valid %v but expected %v", valid, testCase.expectedValid)
 			}
@@ -117,7 +115,8 @@ func TestPruneInvalid(t *testing.T) {
 
 func TestMatch(t *testing.T) {
 	ctx := context.Background()
-	clusterconditions.Register("Error", &Error{})
+	registry := standard.NewConditionRegistry(nil)
+	registry.Register("Error", &Error{})
 
 	for _, testCase := range []struct {
 		name          string
@@ -181,7 +180,7 @@ func TestMatch(t *testing.T) {
 		},
 	} {
 		t.Run(testCase.name, func(t *testing.T) {
-			match, err := clusterconditions.Match(ctx, testCase.conditions)
+			match, err := registry.Match(ctx, testCase.conditions)
 			if match != testCase.expectedMatch {
 				t.Errorf("got match %t but expected %t", match, testCase.expectedMatch)
 			}
@@ -194,6 +193,4 @@ func TestMatch(t *testing.T) {
 			}
 		})
 	}
-
-	delete(clusterconditions.Registry, "Error")
 }

pkg/clusterconditions/mock/mock.go

@@ -44,7 +44,7 @@ type Mock struct {
 }
 
 // Valid returns an error popped from ValidQueue.
-func (m *Mock) Valid(ctx context.Context, condition *configv1.ClusterCondition) error {
+func (m *Mock) Valid(_ context.Context, condition *configv1.ClusterCondition) error {
 	m.Calls = append(m.Calls, Call{
 		When:      time.Now(),
 		Method:    "Valid",
@@ -61,7 +61,7 @@ func (m *Mock) Valid(ctx context.Context, condition *configv1.ClusterCondition)
 }
 
 // Match returns an error popped from MatchQueue.
-func (m *Mock) Match(ctx context.Context, condition *configv1.ClusterCondition) (bool, error) {
+func (m *Mock) Match(_ context.Context, condition *configv1.ClusterCondition) (bool, error) {
 	m.Calls = append(m.Calls, Call{
 		When:      time.Now(),
 		Method:    "Match",