VPC start ip range fix: only generate ip start from range [11, 19]U[2…

…1, 99]

pkg/controller/mizar/mizar-arktos-network-controller.go

@@ -254,10 +254,15 @@ func createVpcAndSubnet(vpc, subnet string, dynamicClient dynamic.Interface) err
 // For the initial release of Arktos Mizar integration, user specified VPC CIDR is not supported
 // Generate VPC CIDR randomly for now.
 func generateVPCSpec(vpcName string) (int, *MizarVPC) {
+	// TODO: this is a quick solution to randomize VPC start ip address. Due to variously reasons, Arktos
+	//   needs randomize VPC start ip to prevent service ip collision for now.
+	// This is a simplified version to avoid reserved internal address - however, it may collision with real external ip address.
+	// Will log as an issue and solve in the future
 	// randomize ip start segment:
-	ipStart := ran.Intn(255) + 1 // IpStart range [1, 255]
-	// Simply not allow ip start from 10, 172, 192, 100 for well-known private range
-	if ipStart == 10 || ipStart == 172 || ipStart == 192 || ipStart == 100 {
+	ipStart := ran.Intn(89) + 11 // IpStart range [11, 99] - 20
+
+	// Exclude 20 as it is used by mizar internally
+	if ipStart == 20 {
 		ipStart++
 	}
 

pkg/controller/mizar/mizar-arktos-network-controller_test.go

@@ -23,31 +23,29 @@ import (
 )
 
 func TestGenerateVPCSpec(t *testing.T) {
-	ipStart, vpcSpec := generateVPCSpec("vpc1")
-	verifyIpStart(t, ipStart)
-	verifyVPCSpec(t, vpcSpec)
+	for i := 0; i < 1000; i++ {
+		ipStart, vpcSpec := generateVPCSpec("vpc1")
+		verifyIpStart(t, ipStart)
+		verifyVPCSpec(t, vpcSpec)
 
-	vpcJsonData, err := json.Marshal(vpcSpec)
-	assert.Nil(t, err, "Unexpected marshalling error")
-	var unmarshallData MizarVPC
-	err = json.Unmarshal(vpcJsonData, &unmarshallData)
-	assert.Nil(t, err, "Unexpected unmarshalling error")
-	assert.Equal(t, vpcSpec.APIVersion, unmarshallData.APIVersion)
-	assert.Equal(t, vpcSpec.Kind, unmarshallData.Kind)
-	assert.Equal(t, vpcSpec.Metadata.Name, unmarshallData.Metadata.Name)
-	assert.Equal(t, vpcSpec.Spec.IP, unmarshallData.Spec.IP)
-	assert.Equal(t, vpcSpec.Spec.Prefix, unmarshallData.Spec.Prefix)
-	assert.Equal(t, vpcSpec.Spec.Divider, unmarshallData.Spec.Divider)
-	assert.Equal(t, vpcSpec.Spec.Status, unmarshallData.Spec.Status)
+		vpcJsonData, err := json.Marshal(vpcSpec)
+		assert.Nil(t, err, "Unexpected marshalling error")
+		var unmarshallData MizarVPC
+		err = json.Unmarshal(vpcJsonData, &unmarshallData)
+		assert.Nil(t, err, "Unexpected unmarshalling error")
+		assert.Equal(t, vpcSpec.APIVersion, unmarshallData.APIVersion)
+		assert.Equal(t, vpcSpec.Kind, unmarshallData.Kind)
+		assert.Equal(t, vpcSpec.Metadata.Name, unmarshallData.Metadata.Name)
+		assert.Equal(t, vpcSpec.Spec.IP, unmarshallData.Spec.IP)
+		assert.Equal(t, vpcSpec.Spec.Prefix, unmarshallData.Spec.Prefix)
+		assert.Equal(t, vpcSpec.Spec.Divider, unmarshallData.Spec.Divider)
+		assert.Equal(t, vpcSpec.Spec.Status, unmarshallData.Spec.Status)
+	}
 }
 
 func verifyIpStart(t *testing.T, ipStart int) {
-	assert.True(t, ipStart >= 1, "VPC started should be in range [1, 255]")
-	assert.True(t, ipStart <= 255, "VPC started should be in range [1, 255]")
-	assert.True(t, ipStart != 10, "VPC cannot start with 10.x.x.x")
-	assert.True(t, ipStart != 172, "VPC cannot start with 172.x.x.x")
-	assert.True(t, ipStart != 192, "VPC cannot start with 192.x.x.x")
-	assert.True(t, ipStart != 100, "VPC cannot start with 100.x.x.x")
+	assert.True(t, ipStart >= 11 && ipStart <= 99 && ipStart != 20, "VPC started should be in range [11, 20) or [21, 99], got %d", ipStart)
+
 }
 
 func verifyVPCSpec(t *testing.T, vpcSpec *MizarVPC) {

pkg/controller/mizar/mizar-service-controller.go

@@ -245,7 +245,7 @@ func (c *MizarServiceController) processServiceCreateOrUpdate(service *v1.Servic
 		service.Annotations[mizarAnnotationsVpcKey] = getVPC(tenantDefaultNetwork)
 		service.Annotations[mizarAnnotationsSubnetKey] = getSubnetNameFromVPC(tenantDefaultNetwork.Spec.VPCID)
 		_, err := c.kubeClientset.CoreV1().ServicesWithMultiTenancy(service.Namespace, service.Tenant).Update(service)
-		klog.V(4).Infof("Add mizar annotation for service %s/%s/%s. error %v", key, err)
+		klog.V(4).Infof("Add mizar annotation for service %s. error %v", key, err)
 		if err != nil {
 			return errors.New(fmt.Sprintf("update service %s mizar annotation got error (%v)", key, err))
 		}
@@ -303,7 +303,7 @@ func (c *MizarServiceController) processServiceCreateOrUpdate(service *v1.Servic
 			}
 		}
 	case CodeType_TEMP_ERROR:
-		klog.Warningf("Mizar hit temporary error for service creation for service: %s.")
+		klog.Warningf("Mizar hit temporary error for service creation for service: %s.", key)
 		return errors.New("Service creation failed on mizar side, will try again.....")
 	case CodeType_PERM_ERROR:
 		klog.Errorf("Mizar hit permanent error for service creation for service: %s.", key)
@@ -323,7 +323,7 @@ func (c *MizarServiceController) processServiceCreateOrUpdate(service *v1.Servic
 			return err
 		}
 	} else if service.Spec.ClusterIP != ip {
-		klog.Warningf("Service %s cluster ip %s is different from mizar assigned ip %s", key, ip)
+		klog.Warningf("Service %s cluster ip %s is different from mizar assigned ip %s", key, service.Spec.ClusterIP, ip)
 	}
 
 	return nil