Merge pull request #431 from deads2k/internal-lb

use internal LB to avoid outages during kube-apiserver rollout

pkg/operator/configobservation/configobservercontroller/observe_config_controller.go

@@ -1,6 +1,7 @@
 package configobservercontroller
 
 import (
+	"github.com/openshift/cluster-kube-controller-manager-operator/pkg/operator/configobservation/masterurl"
 	"k8s.io/client-go/tools/cache"
 
 	configinformers "github.com/openshift/client-go/config/informers/externalversions"
@@ -93,6 +94,7 @@ func NewConfigObserver(
 			proxy.NewProxyObserveFunc([]string{"targetconfigcontroller", "proxy"}),
 			serviceca.ObserveServiceCA,
 			clustername.ObserveInfraID,
+			masterurl.ObserveMasterURL,
 		),
 	}
 

pkg/operator/configobservation/masterurl/observe_masterurl.go

@@ -0,0 +1,45 @@
+package masterurl
+
+import (
+	"k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+
+	configv1 "github.com/openshift/api/config/v1"
+	"github.com/openshift/cluster-kube-controller-manager-operator/pkg/operator/configobservation"
+	"github.com/openshift/library-go/pkg/operator/configobserver"
+	"github.com/openshift/library-go/pkg/operator/events"
+)
+
+// ObserveMasterURL fills in the cluster-name extended argument for the controller-manager with the cluster's infra ID
+func ObserveMasterURL(genericListers configobserver.Listers, recorder events.Recorder, existingConfig map[string]interface{}) (map[string]interface{}, []error) {
+	listers := genericListers.(configobservation.Listers)
+	errs := []error{}
+	masterURLPath := []string{"extendedArguments", "master"}
+	previouslyObservedConfig := map[string]interface{}{}
+
+	if currentMasterURL, _, _ := unstructured.NestedStringSlice(existingConfig, masterURLPath...); len(currentMasterURL) > 0 {
+		if err := unstructured.SetNestedStringSlice(previouslyObservedConfig, currentMasterURL, masterURLPath...); err != nil {
+			errs = append(errs, err)
+		}
+	}
+
+	observedConfig := map[string]interface{}{}
+	infrastructure, err := listers.InfrastructureLister().Get("cluster")
+	if err != nil {
+		if errors.IsNotFound(err) {
+			recorder.Warningf("ObserveMasterURL", "Required infrastructures.%s/cluster not found", configv1.GroupName)
+		}
+		return previouslyObservedConfig, errs
+	}
+	// The infrastructureName value in infrastructure status is always present and cannot be changed during the
+	// lifetime of the cluster.
+	newMasterURL := infrastructure.Status.APIServerInternalURL
+	if len(newMasterURL) == 0 {
+		recorder.Warningf("ObserveMasterURL", "Value for infrastructureName in infrastructure.%s/cluster is blank", configv1.GroupName)
+		return previouslyObservedConfig, errs
+	}
+	if err := unstructured.SetNestedStringSlice(observedConfig, []string{newMasterURL}, masterURLPath...); err != nil {
+		errs = append(errs, err)
+	}
+	return observedConfig, errs
+}

pkg/operator/configobservation/masterurl/observe_masterurl_test.go

@@ -0,0 +1,118 @@
+package masterurl
+
+import (
+	"reflect"
+	"testing"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/tools/cache"
+
+	configv1 "github.com/openshift/api/config/v1"
+	configlistersv1 "github.com/openshift/client-go/config/listers/config/v1"
+	"github.com/openshift/library-go/pkg/operator/events"
+
+	"github.com/ghodss/yaml"
+
+	"github.com/openshift/cluster-kube-controller-manager-operator/pkg/operator/configobservation"
+)
+
+func TestObserveInfraID(t *testing.T) {
+	type Test struct {
+		name            string
+		config          *configv1.Infrastructure
+		input, expected map[string]interface{}
+	}
+	tests := []Test{
+		{
+			name: "new name, no old config",
+			config: &configv1.Infrastructure{
+				ObjectMeta: metav1.ObjectMeta{Name: "cluster"},
+				Status: configv1.InfrastructureStatus{
+					APIServerInternalURL: "newClusterName",
+				},
+			},
+			input: map[string]interface{}{},
+			expected: map[string]interface{}{
+				"extendedArguments": map[string]interface{}{
+					"master": []interface{}{
+						"newClusterName",
+					},
+				},
+			},
+		},
+		{
+			name: "new name, old config",
+			config: &configv1.Infrastructure{
+				ObjectMeta: metav1.ObjectMeta{Name: "cluster"},
+				Status: configv1.InfrastructureStatus{
+					APIServerInternalURL: "newClusterName",
+				},
+			},
+			input: map[string]interface{}{
+				"extendedArguments": map[string]interface{}{
+					"master": []interface{}{
+						"oldClusterName",
+					},
+				},
+			},
+			expected: map[string]interface{}{
+				"extendedArguments": map[string]interface{}{
+					"master": []interface{}{
+						"newClusterName",
+					},
+				},
+			},
+		},
+		{
+			name:     "none, no old config",
+			config:   &configv1.Infrastructure{ObjectMeta: metav1.ObjectMeta{Name: "cluster"}},
+			input:    map[string]interface{}{},
+			expected: map[string]interface{}{},
+		},
+		{
+			name:   "none, existing config",
+			config: &configv1.Infrastructure{ObjectMeta: metav1.ObjectMeta{Name: "cluster"}},
+			input: map[string]interface{}{
+				"extendedArguments": map[string]interface{}{
+					"master": []interface{}{
+						"oldClusterName",
+					},
+				},
+			},
+			expected: map[string]interface{}{
+				"extendedArguments": map[string]interface{}{
+					"master": []interface{}{
+						"oldClusterName",
+					},
+				},
+			},
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{})
+			if err := indexer.Add(test.config); err != nil {
+				t.Fatal(err.Error())
+			}
+			listers := configobservation.Listers{
+				InfrastructureLister_: configlistersv1.NewInfrastructureLister(indexer),
+			}
+			result, errs := ObserveMasterURL(listers, events.NewInMemoryRecorder("infraid"), test.input)
+			if len(errs) > 0 {
+				t.Fatal(errs)
+			} else {
+				if !reflect.DeepEqual(test.expected, result) {
+					t.Errorf("\n===== observed config expected:\n%v\n===== observed config actual:\n%v", toYAML(test.expected), toYAML(result))
+				}
+			}
+		})
+	}
+}
+
+func toYAML(o interface{}) string {
+	b, e := yaml.Marshal(o)
+	if e != nil {
+		return e.Error()
+	}
+	return string(b)
+}