Code review changes.

Author: templecloud

pkg/oci/load_balancer.go

@@ -324,7 +324,6 @@ func (cp *CloudProvider) EnsureLoadBalancer(ctx context.Context, clusterName str
 	}
 	subnets := []string{cp.config.LoadBalancer.Subnet1, cp.config.LoadBalancer.Subnet2}
 	spec, err := NewLBSpec(service, nodes, subnets, ssl, cp.securityListManagerFactory)
-
 	if err != nil {
 		logger.With(zap.Error(err)).Error("Failed to derive LBSpec")
 		return nil, err

pkg/oci/load_balancer_security_lists.go

@@ -152,9 +152,9 @@ func (s *baseSecurityListManager) updateLoadBalancerRules(ctx context.Context, l
 
 		logger := s.logger.With("securityListID", *secList.Id)
 
-		// -1 denotes nil ports.
-		var currentBackEndPort = -1
-		var currentHealthCheck = -1
+		// 0 denotes nil ports.
+		var currentBackEndPort = 0
+		var currentHealthCheck = 0
 		if actualPorts != nil {
 			currentBackEndPort = actualPorts.BackendPort
 			currentHealthCheck = actualPorts.HealthCheckerPort
@@ -360,9 +360,9 @@ func getNodeIngressRules(
 	desiredPorts portSpec,
 	serviceLister listersv1.ServiceLister,
 ) []core.IngressSecurityRule {
-	// -1 denotes nil ports.
-	var currentBackEndPort = -1
-	var currentHealthCheckPort = -1
+	// 0 denotes nil ports.
+	var currentBackEndPort = 0
+	var currentHealthCheckPort = 0
 	if actualPorts != nil {
 		currentBackEndPort = actualPorts.BackendPort
 		currentHealthCheckPort = actualPorts.HealthCheckerPort

test/e2e/README.md

@@ -5,13 +5,32 @@ E2E tests.
 
 ## Running
 
-
 ```bash
 $ ginkgo -v -progress test/e2e -- --kubeconfig=${HOME}/.kube/config --delete-namespace=false
 ```
 
 NOTE: Test suite will fail if executed behind a `$HTTP_PROXY` that returns a
 200 OK response upon failure to connect.
 
+## Additional Options
+
+Additional seclist count based sanity checks can be applied during e2e testing 
+by providing the appropriate seclist ocids. Both must be supplied.
+
+```bash
+export CCM_SECLIST_ID="ocid1.securitylist.$ccmloadblancerid"
+export K8S_SECLIST_ID="ocid1.securitylist.$k8sworkerid"
+```
+
+Alternatively, these values can be specified as command line parameters.
+
+```bash
+$ ginkgo -v -progress test/e2e -- \
+    --kubeconfig=${HOME}/.kube/config \
+    --delete-namespace=false \
+    --ccm-seclist-id=ocid1.securitylist.$ccmloadblancerid \
+    --k8s-seclist-id=ocid1.securitylist.$k8sworkerid
+```
+
 
 [1]: https://github.com/kubernetes/kubernetes/blob/0cb15453dae92d8be66cf42e6c1b04e21a2d0fb6/test/e2e/network/service.go

test/e2e/framework/framework.go

@@ -51,12 +51,16 @@ var (
 	kubeconfig      string // path to kubeconfig file
 	deleteNamespace bool   // whether or not to delete test namespaces
 	cloudConfigFile string // path to cloud provider config file
+	ccmSeclistID    string // The ocid of the loadbalancer subnet seclist. Optional.
+	k8sSeclistID    string // The ocid of the k8s worker subnet seclist. Optional.
 )
 
 func init() {
 	flag.StringVar(&kubeconfig, "kubeconfig", "", "Path to Kubeconfig file with authorization and master location information.")
 	flag.BoolVar(&deleteNamespace, "delete-namespace", true, "If true tests will delete namespace after completion. It is only designed to make debugging easier, DO NOT turn it off by default.")
 	flag.StringVar(&cloudConfigFile, "cloud-config", "", "The path to the cloud provider configuration file. Empty string for no configuration file.")
+	flag.StringVar(&ccmSeclistID, "ccm-seclist-id", "", "The ocid of the loadbalancer subnet seclist. Enables additional seclist rule tests. If specified the 'k8s-seclist-id parameter' is also required.")
+	flag.StringVar(&k8sSeclistID, "k8s-seclist-id", "", "The ocid of the k8s worker subnet seclist. Enables additional seclist rule tests. If specified the 'ccm-seclist-id parameter' is also required.")
 }
 
 // Framework is used in the execution of e2e tests.
@@ -109,8 +113,14 @@ func NewFramework(baseName string, client clientset.Interface) *Framework {
 	}
 	// Dev/CI only configuration. The seclist for CCM load-balancer routes.
 	f.CCMSecListID = os.Getenv("CCM_SECLIST_ID")
+	if ccmSeclistID != "" {
+		f.CCMSecListID = ccmSeclistID // Commandline override.
+	}
 	// Dev/CI only configuration. The seclist for K8S worker node routes.
 	f.K8SSecListID = os.Getenv("K8S_SECLIST_ID")
+	if k8sSeclistID != "" {
+		f.K8SSecListID = k8sSeclistID // Commandline override.
+	}
 	BeforeEach(f.BeforeEach)
 	AfterEach(f.AfterEach)
 
@@ -252,16 +262,17 @@ func (f *Framework) AfterEach() {
 	}
 }
 
-// createCloudProviderConfig unmarshalls the CCM's cloud provider config from the specified location so it can be used for testing.
+// createCloudProviderConfig unmarshalls the CCM's cloud provider config from
+// the specified location so it can be used for testing.
 func createCloudProviderConfig(cloudConfigFile string) (*oci.Config, error) {
 	file, err := os.Open(cloudConfigFile)
 	if err != nil {
-		return nil, errors.Wrap(err, fmt.Sprintf("Couldn't open cloud provider configuration: %s.", cloudConfigFile))
+		return nil, errors.Wrapf(err, "Couldn't open cloud provider configuration: %s.", cloudConfigFile)
 	}
 	defer file.Close()
 	cloudProviderConfig, err := oci.ReadConfig(file)
 	if err != nil {
-		return nil, errors.Wrap(err, fmt.Sprintf("Couldn't create cloud provider configuration: %s.", cloudConfigFile))
+		return nil, errors.Wrapf(err, "Couldn't create cloud provider configuration: %s.", cloudConfigFile)
 	}
 	return cloudProviderConfig, nil
 }
@@ -274,7 +285,7 @@ func createOCIClient(cloudProviderConfig *oci.Config) (client.Interface, error)
 	rateLimiter := oci.NewRateLimiter(logger.Sugar(), cloudProviderConfig.RateLimiter)
 	ociClient, err := client.New(logger.Sugar(), ociClientConfig, &rateLimiter)
 	if err != nil {
-		return nil, errors.Wrap(err, fmt.Sprintf("Couldn't create oci client from configuration: %s.", cloudConfigFile))
+		return nil, errors.Wrapf(err, "Couldn't create oci client from configuration: %s.", cloudConfigFile)
 	}
 	return ociClient, nil
 }

test/e2e/framework/seclist_util.go

@@ -24,9 +24,9 @@ import (
 	core "github.com/oracle/oci-go-sdk/core"
 )
 
-// CountSecListTunnelSinglePortRules counts the number of 'single port'
+// CountSinglePortSecListRules counts the number of 'single port'
 // (non-ranged) egress/ingress rules for the specified list and port.
-func CountSecListTunnelSinglePortRules(oci client.Interface, egressSecListID, ingressSecListID string, port int) (int, int) {
+func CountSinglePortSecListRules(oci client.Interface, egressSecListID, ingressSecListID string, port int) (int, int) {
 	numEgressRules := CountEgressSinglePortRules(oci, egressSecListID, port)
 	numIngressRules := CountIngressSinglePortRules(oci, ingressSecListID, port)
 	return numEgressRules, numIngressRules

test/e2e/load_balancer.go

@@ -95,7 +95,7 @@ var _ = Describe("Service [Slow]", func() {
 
 		// Count the number of ingress/egress rules with the original port so
 		// we can check the correct number are updated.
-		numEgressRules, numIngressRules := framework.CountSecListTunnelSinglePortRules(f.Client, f.CCMSecListID, f.K8SSecListID, tcpNodePort)
+		numEgressRules, numIngressRules := framework.CountSinglePortSecListRules(f.Client, f.CCMSecListID, f.K8SSecListID, tcpNodePort)
 		tcpService = jig.ChangeServiceNodePortOrFail(ns, tcpService.Name, tcpNodePort)
 		jig.SanityCheckService(tcpService, v1.ServiceTypeLoadBalancer)
 

wercker.yml

@@ -138,6 +138,8 @@ e2e-test:
       code: |
         export VERSION=$(cat VERSION.txt)
         echo "${VERSION}"
+        export CCM_SECLIST_ID="ocid1.securitylist.oc1.iad.aaaaaaaaqshdqfwpgqnvt42vrvtn4oqlpxvmgte5r5j7aczkxghodftx77gq"
+        export K8S_SECLIST_ID="ocid1.securitylist.oc1.iad.aaaaaaaazsac74oe2fml7bhpmkbboik7zfzsma2eakeummgeyvbuzpjbvs4a"
 
     - script:
       name: deploy latest CCM