Merge pull request #31 from aws/feature/failure-domains

feature/failure domains

Author: rejoshed

.gitignore

@@ -6,6 +6,10 @@
 *.dylib
 bin
 testbin/*
+api/*/zz*generated.deepcopy.go
+
+nginx.conf
+nginx.conf.bak
 
 pkg/mocks/*
 !pkg/mocks/.keep

Makefile

@@ -74,16 +74,12 @@ api/%/zz_generated.deepcopy.go: bin/controller-gen $(DEEPCOPY_GEN_INPUTS)
 
 MANAGER_BIN_INPUTS=$(shell find ./controllers ./api ./pkg -name "*mock*" -prune -o -name "*test*" -prune -o -type f -print) main.go go.mod go.sum
 .PHONY: build
-build: binaries generate-mocks generate-deepcopy manifests release-manifests ## Build manager binary.
+build: binaries generate-deepcopy lint manifests release-manifests ## Build manager binary.
 bin/manager: $(MANAGER_BIN_INPUTS)
-	go fmt ./...
-	go vet ./...
 	go build -o bin/manager main.go
 
 .PHONY: run
-run: generate-deepcopy fmt vet ## Run a controller from your host.
-	go fmt ./...
-	go vet ./...
+run: generate-deepcopy ## Run a controller from your host.
 	go run ./main.go
 
 # Using a flag file here as docker build doesn't produce a target file.
@@ -163,14 +159,14 @@ clean: ## Clean.
 export KUBEBUILDER_ASSETS=$(PROJECT_DIR)/bin
 
 .PHONY: test
-test: generate-mocks lint generate-deepcopy bin/ginkgo bin/kubectl bin/kube-apiserver bin/etcd ## Run tests. At the moment this is only unit tests.
+test: generate-mocks lint bin/ginkgo bin/kubectl bin/kube-apiserver bin/etcd ## Run tests. At the moment this is only unit tests.
 	@./hack/testing_ginkgo_recover_statements.sh --add # Add ginkgo.GinkgoRecover() statements to controllers.
 	@# The following is a slightly funky way to make sure the ginkgo statements are removed regardless the test results.
 	@ginkgo -v ./api/... ./controllers/... ./pkg/... -coverprofile cover.out; EXIT_STATUS=$$?;\
 		./hack/testing_ginkgo_recover_statements.sh --remove; exit $$EXIT_STATUS
 
 .PHONY: generate-mocks
-generate-mocks: bin/mockgen pkg/mocks/mock_client.go $(shell find ./pkg/mocks -type f -name "mock*.go") ## Generate mocks needed for testing. Primarily mocks of the cloud package.
+generate-mocks: bin/mockgen generate-deepcopy pkg/mocks/mock_client.go $(shell find ./pkg/mocks -type f -name "mock*.go") ## Generate mocks needed for testing. Primarily mocks of the cloud package.
 pkg/mocks/mock%.go: $(shell find ./pkg/cloud -type f -name "*test*" -prune -o -print)
 	go generate ./...
 

api/v1beta1/cloudstackcluster_types.go

@@ -39,13 +39,58 @@ type CloudStackIdentityReference struct {
 	Name string `json:"name"`
 }
 
+type Network struct {
+	// Cloudstack Network ID the cluster is built in.
+	// +optional
+	ID string `json:"id,omitempty"`
+
+	// Cloudstack Network Type the cluster is built in.
+	// + optional
+	Type string `json:"type,omitempty"`
+
+	// Cloudstack Network Name the cluster is built in.
+	// +optional
+	Name string `json:"name"`
+}
+
+type ZoneStatusMap map[string]Zone
+
+// GetOne just returns a Zone from the map of zone statuses
+// Needed as there's no short way to do this.
+func (zones ZoneStatusMap) GetOne() *Zone {
+	for _, zone := range zones {
+		return &zone
+	}
+	return nil
+}
+
+// GetByName fetches a zone by name if present in the map of zone statuses.
+// Needed as there's no short way to do this.
+func (zones ZoneStatusMap) GetByName(name string) *Zone {
+	for zoneName, zone := range zones {
+		if zoneName == name {
+			return &zone
+		}
+	}
+	return nil
+}
+
+type Zone struct {
+	// The Zone name.
+	// + optional
+	Name string `json:"name,omitempty"`
+
+	// The CS zone ID the cluster is built in.
+	// + optional
+	ID string `json:"id,omitempty"`
+
+	// The network within the Zone to use.
+	Network Network `json:"network"`
+}
+
 // CloudStackClusterSpec defines the desired state of CloudStackCluster.
 type CloudStackClusterSpec struct {
-	// CloudStack Zone name.
-	Zone string `json:"zone"`
-
-	// CloudStack guest network name.
-	Network string `json:"network,omitempty"`
+	Zones []Zone `json:"zones"`
 
 	// The kubernetes control plane endpoint.
 	ControlPlaneEndpoint clusterv1.APIEndpoint `json:"controlPlaneEndpoint"`
@@ -65,24 +110,28 @@ type CloudStackClusterSpec struct {
 
 // The status of the abstract CS k8s (not an actual Cloudstack Cluster) cluster.
 type CloudStackClusterStatus struct {
-	// Reflects the readiness of the CS cluster.
-	Ready bool `json:"ready"`
 
-	// The CS zone ID the cluster is built in.
-	ZoneID string `json:"zoneID"`
+	// The status of the cluster's ACS Zones.
+	// +optional
+	Zones ZoneStatusMap `json:"zones,omitempty"`
 
-	// Cloudstack Network ID the cluster is built in.
-	NetworkID string `json:"networkID,omitempty"`
+	// CAPI recognizes failure domains as a method to spread machines.
+	// CAPC sets failure domains to indicate functioning Zones.
+	// +optional
+	FailureDomains clusterv1.FailureDomains `json:"failureDomains,omitempty"`
 
-	// Cloudstack Network Type the cluster is built in.
-	NetworkType string `json:"networkType,omitempty"`
+	// Reflects the readiness of the CS cluster.
+	Ready bool `json:"ready"`
 
 	// Cloudstack Domain ID the cluster is built in.
 	DomainID string `json:"domainID,omitempty"`
 
 	// The CS public IP ID to use for the k8s endpoint.
 	PublicIPID string `json:"publicIPID,omitempty"`
 
+	// The ID of the network the PublicIP is in.
+	PublicIPNetworkID string `json:"publicIPNetworkID,omitempty"`
+
 	// The ID of the lb rule used to assign VMs to the lb.
 	LBRuleID string `json:"loadBalancerRuleID,omitempty"`
 }

api/v1beta1/cloudstackcluster_webhook.go

@@ -18,6 +18,7 @@ package v1beta1
 
 import (
 	"fmt"
+	"reflect"
 
 	"github.com/aws/cluster-api-provider-cloudstack/pkg/webhookutil"
 	"k8s.io/apimachinery/pkg/api/errors"
@@ -43,7 +44,7 @@ var _ webhook.Defaulter = &CloudStackCluster{}
 
 // Default implements webhook.Defaulter so a webhook will be registered for the type
 func (r *CloudStackCluster) Default() {
-	cloudstackclusterlog.Info("default", "name", r.Name)
+	cloudstackclusterlog.V(1).Info("entered api default setting webhook", "api resource name", r.Name)
 	// No defaulted values supported yet.
 }
 
@@ -53,7 +54,7 @@ var _ webhook.Validator = &CloudStackCluster{}
 
 // ValidateCreate implements webhook.Validator so a webhook will be registered for the type
 func (r *CloudStackCluster) ValidateCreate() error {
-	cloudstackclusterlog.Info("validate create", "name", r.Name)
+	cloudstackclusterlog.V(1).Info("entered validate create webhook", "api resource name", r.Name)
 
 	var errorList field.ErrorList
 
@@ -63,23 +64,31 @@ func (r *CloudStackCluster) ValidateCreate() error {
 	}
 
 	if (r.Spec.Account != "") && (r.Spec.Domain == "") {
-		errorList = append(errorList, field.Required(field.NewPath("spec", "account"), "specifying account requires additionally specifying domain"))
+		errorList = append(errorList, field.Required(
+			field.NewPath("spec", "account"), "specifying account requires additionally specifying domain"))
 	}
 
-	// Zone and Network are required fields
-	errorList = webhookutil.EnsureFieldExists(r.Spec.Zone, "Zone", errorList)
-	errorList = webhookutil.EnsureFieldExists(r.Spec.Network, "Network", errorList)
+	// Require Zones and their respective Networks.
+	if len(r.Spec.Zones) <= 0 {
+		errorList = append(errorList, field.Required(field.NewPath("spec", "Zones"), "Zones"))
+	} else {
+		for _, zone := range r.Spec.Zones {
+			if zone.Network.Name == "" && zone.Network.ID == "" {
+				errorList = append(errorList, field.Required(
+					field.NewPath("spec", "Zones", "Network"), "each Zone requires a Network specification"))
+			}
+		}
+	}
 
 	return webhookutil.AggregateObjErrors(r.GroupVersionKind().GroupKind(), r.Name, errorList)
 }
 
 // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
 func (r *CloudStackCluster) ValidateUpdate(old runtime.Object) error {
-	cloudstackclusterlog.Info("validate update", "name", r.Name)
+	cloudstackclusterlog.V(1).Info("entered validate update webhook", "api resource name", r.Name)
 
 	var (
-		errorList field.ErrorList
-		spec      = r.Spec
+		spec = r.Spec
 	)
 
 	oldCluster, ok := old.(*CloudStackCluster)
@@ -88,31 +97,37 @@ func (r *CloudStackCluster) ValidateUpdate(old runtime.Object) error {
 	}
 	oldSpec := oldCluster.Spec
 
-	// IdentityRefs must be Secrets.
-	if spec.IdentityRef != nil && spec.IdentityRef.Kind != defaultIdentityRefKind {
-		errorList = append(errorList, field.Forbidden(field.NewPath("spec", "identityRef", "kind"), "must be a Secret"))
+	// No spec fields may be updated.
+	errorList := field.ErrorList(nil)
+	if !reflect.DeepEqual(oldSpec.Zones, spec.Zones) {
+		errorList = append(errorList, field.Forbidden(
+			field.NewPath("spec", "Zones"), "Zones and sub-attributes may not be modified after creation"))
 	}
-
-	// No spec fields may be changed
-	errorList = webhookutil.EnsureStringFieldsAreEqual(spec.Zone, oldSpec.Zone, "zone", errorList)
-	errorList = webhookutil.EnsureStringFieldsAreEqual(spec.Network, oldSpec.Network, "network", errorList)
 	if oldSpec.ControlPlaneEndpoint.Host != "" { // Need to allow one time endpoint setting via CAPC cluster controller.
 		errorList = webhookutil.EnsureStringFieldsAreEqual(
-			spec.ControlPlaneEndpoint.Host, oldSpec.ControlPlaneEndpoint.Host, "controlplaneendpointhost", errorList)
+			spec.ControlPlaneEndpoint.Host, oldSpec.ControlPlaneEndpoint.Host, "controlplaneendpoint.host", errorList)
 		errorList = webhookutil.EnsureStringFieldsAreEqual(
-			string(spec.ControlPlaneEndpoint.Port), string(oldSpec.ControlPlaneEndpoint.Port), "controlplaneendpointport", errorList)
+			string(spec.ControlPlaneEndpoint.Port), string(oldSpec.ControlPlaneEndpoint.Port),
+			"controlplaneendpoint.port", errorList)
 	}
 	if spec.IdentityRef != nil && oldSpec.IdentityRef != nil {
-		errorList = webhookutil.EnsureStringFieldsAreEqual(spec.IdentityRef.Kind, oldSpec.IdentityRef.Kind, "identityRef.Kind", errorList)
-		errorList = webhookutil.EnsureStringFieldsAreEqual(spec.IdentityRef.Name, oldSpec.IdentityRef.Name, "identityRef.Name", errorList)
+		errorList = webhookutil.EnsureStringFieldsAreEqual(
+			spec.IdentityRef.Kind, oldSpec.IdentityRef.Kind, "identityref.kind", errorList)
+		errorList = webhookutil.EnsureStringFieldsAreEqual(spec.IdentityRef.Name, oldSpec.IdentityRef.Name,
+			"identityref.name", errorList)
+	}
+
+	// IdentityRefs must be Secrets.
+	if spec.IdentityRef != nil && spec.IdentityRef.Kind != defaultIdentityRefKind {
+		errorList = append(errorList, field.Forbidden(field.NewPath("spec", "identityRef", "kind"), "must be a Secret"))
 	}
 
 	return webhookutil.AggregateObjErrors(r.GroupVersionKind().GroupKind(), r.Name, errorList)
 }
 
 // ValidateDelete implements webhook.Validator so a webhook will be registered for the type
 func (r *CloudStackCluster) ValidateDelete() error {
-	cloudstackclusterlog.Info("validate delete", "name", r.Name)
+	cloudstackclusterlog.V(1).Info("entered validate delete webhook", "api resource name", r.Name)
 	// No deletion validations.  Deletion webhook not enabled.
 	return nil
 }