Set network config status even with unknown network plugin

Some components depend on some fields of configv1.NetworkStatus. Eg,
cluster-kubernetes-apiserver-operator needs to know the
ServiceNetwork.

Previously we weren't setting the status when the default network type
was unknown, because we didn't know that the external network plugin
would consider them valid. (eg, there might be multiple ClusterNetwork
values but the plugin only supports one.) But this breaks components
that assume they can get their config from there.

Fix this by setting the status fields but allowing them to be
overridden by another operator if the network type is unknown.

pkg/controller/operconfig/cluster.go

@@ -81,7 +81,7 @@ func (r *ReconcileOperConfig) ClusterNetworkStatus(ctx context.Context, operConf
 	}
 
 	// Update the cluster config status
-	status := network.StatusFromOperatorConfig(&operConfig.Spec)
+	status := network.StatusFromOperatorConfig(&operConfig.Spec, &clusterConfig.Status)
 	if status == nil || reflect.DeepEqual(*status, clusterConfig.Status) {
 		return nil, nil
 	}

pkg/network/cluster_config.go

@@ -84,8 +84,10 @@ func MergeClusterConfig(operConf *operv1.NetworkSpec, clusterConf configv1.Netwo
 
 // StatusFromOperatorConfig generates the cluster NetworkStatus from the
 // currently applied operator configuration.
-func StatusFromOperatorConfig(operConf *operv1.NetworkSpec) *configv1.NetworkStatus {
-	// Don't set status if we don't understand the network type
+func StatusFromOperatorConfig(operConf *operv1.NetworkSpec, oldStatus *configv1.NetworkStatus) *configv1.NetworkStatus {
+	knownNetworkType := true
+	status := configv1.NetworkStatus{}
+
 	switch operConf.DefaultNetwork.Type {
 	case operv1.NetworkTypeOpenShiftSDN:
 		// continue
@@ -94,22 +96,29 @@ func StatusFromOperatorConfig(operConf *operv1.NetworkSpec) *configv1.NetworkSta
 	case operv1.NetworkTypeKuryr:
 		// continue
 	default:
-		return nil
+		knownNetworkType = false
+		// Preserve any status fields set by the unknown network plugin
+		status = *oldStatus
+	}
+
+	if oldStatus.NetworkType == "" || knownNetworkType {
+		status.NetworkType = string(operConf.DefaultNetwork.Type)
 	}
 
 	// TODO: when we support expanding the service cidr or cluster cidr,
 	// don't actually update the status until the changes are rolled out.
-	status := configv1.NetworkStatus{
-		ServiceNetwork: operConf.ServiceNetwork,
-		NetworkType:    string(operConf.DefaultNetwork.Type),
-	}
 
-	for _, cnet := range operConf.ClusterNetwork {
-		status.ClusterNetwork = append(status.ClusterNetwork,
-			configv1.ClusterNetworkEntry{
-				CIDR:       cnet.CIDR,
-				HostPrefix: cnet.HostPrefix,
-			})
+	if len(oldStatus.ServiceNetwork) == 0 || knownNetworkType {
+		status.ServiceNetwork = operConf.ServiceNetwork
+	}
+	if len(oldStatus.ClusterNetwork) == 0 || knownNetworkType {
+		for _, cnet := range operConf.ClusterNetwork {
+			status.ClusterNetwork = append(status.ClusterNetwork,
+				configv1.ClusterNetworkEntry{
+					CIDR:       cnet.CIDR,
+					HostPrefix: cnet.HostPrefix,
+				})
+		}
 	}
 
 	// Determine the MTU from the provider

pkg/network/cluster_config_test.go

@@ -103,7 +103,7 @@ func TestStatusFromConfig(t *testing.T) {
 	var mtu uint32 = 1300
 	crd.Spec.DefaultNetwork.OpenShiftSDNConfig.MTU = &mtu
 
-	status := StatusFromOperatorConfig(&crd.Spec)
+	status := StatusFromOperatorConfig(&crd.Spec, &configv1.NetworkStatus{})
 	g.Expect(status).To(Equal(&configv1.NetworkStatus{
 		ClusterNetwork: []configv1.ClusterNetworkEntry{
 			{
@@ -121,7 +121,92 @@ func TestStatusFromConfig(t *testing.T) {
 		NetworkType: "OpenShiftSDN",
 	}))
 
-	crd.Spec.DefaultNetwork.Type = "nasa"
-	status = StatusFromOperatorConfig(&crd.Spec)
-	g.Expect(status).To(BeNil())
+	*crd.Spec.DefaultNetwork.OpenShiftSDNConfig.MTU = 1500
+	status = StatusFromOperatorConfig(&crd.Spec, status)
+	g.Expect(status).To(Equal(&configv1.NetworkStatus{
+		ClusterNetwork: []configv1.ClusterNetworkEntry{
+			{
+				CIDR:       "10.128.0.0/15",
+				HostPrefix: 23,
+			},
+			{
+				CIDR:       "10.0.0.0/14",
+				HostPrefix: 24,
+			},
+		},
+		ServiceNetwork:    []string{"172.30.0.0/16"},
+		ClusterNetworkMTU: 1500,
+
+		NetworkType: "OpenShiftSDN",
+	}))
+
+	// If someone manually edits the status we will overwrite them
+	status.ClusterNetwork = status.ClusterNetwork[:1]
+	status.ServiceNetwork = []string{"172.30.0.0/17"}
+	status.ClusterNetworkMTU = 1450
+
+	status = StatusFromOperatorConfig(&crd.Spec, status)
+	g.Expect(status).To(Equal(&configv1.NetworkStatus{
+		ClusterNetwork: []configv1.ClusterNetworkEntry{
+			{
+				CIDR:       "10.128.0.0/15",
+				HostPrefix: 23,
+			},
+			{
+				CIDR:       "10.0.0.0/14",
+				HostPrefix: 24,
+			},
+		},
+		ServiceNetwork:    []string{"172.30.0.0/16"},
+		ClusterNetworkMTU: 1500,
+
+		NetworkType: "OpenShiftSDN",
+	}))
+}
+
+func TestStatusFromConfigUnknown(t *testing.T) {
+	g := NewGomegaWithT(t)
+
+	crd := OpenShiftSDNConfig.DeepCopy()
+	FillDefaults(&crd.Spec, nil)
+
+	crd.Spec.DefaultNetwork.Type = "None"
+
+	status := StatusFromOperatorConfig(&crd.Spec, &configv1.NetworkStatus{})
+	g.Expect(status).To(Equal(&configv1.NetworkStatus{
+		ClusterNetwork: []configv1.ClusterNetworkEntry{
+			{
+				CIDR:       "10.128.0.0/15",
+				HostPrefix: 23,
+			},
+			{
+				CIDR:       "10.0.0.0/14",
+				HostPrefix: 24,
+			},
+		},
+		ServiceNetwork:    []string{"172.30.0.0/16"},
+		ClusterNetworkMTU: 0,
+
+		NetworkType: "None",
+	}))
+
+	// The external network plugin updates the status itself...
+	status.ClusterNetwork = status.ClusterNetwork[:1]
+	status.ServiceNetwork = []string{"172.30.0.0/17"}
+	status.ClusterNetworkMTU = 1450
+
+	// The external changes should be preserved
+	status = StatusFromOperatorConfig(&crd.Spec, status)
+	g.Expect(status).To(Equal(&configv1.NetworkStatus{
+		ClusterNetwork: []configv1.ClusterNetworkEntry{
+			{
+				CIDR:       "10.128.0.0/15",
+				HostPrefix: 23,
+			},
+		},
+		ServiceNetwork:    []string{"172.30.0.0/17"},
+		ClusterNetworkMTU: 1450,
+
+		NetworkType: "None",
+	}))
 }