🏃 Remove all TODOs in the codebase

Makefile

@@ -177,7 +177,6 @@ generate-go: $(CONTROLLER_GEN) $(MOCKGEN) $(CONVERSION_GEN) ## Runs Go related g
 		paths=./api/... \
 		paths=./$(EXP_DIR)/api/... \
 		object:headerFile=./hack/boilerplate/boilerplate.generatego.txt
-
 	$(CONVERSION_GEN) \
 		--input-dirs=./api/v1alpha2 \
 		--output-file-base=zz_generated.conversion \

README.md

@@ -19,11 +19,7 @@ hybrid deployments of Kubernetes.
 
 Check out the [Cluster API Quick Start][quickstart] to create your first Kubernetes cluster on Azure using Cluster API.
 
-## Features
-
-TODO
-
----
+------
 
 ## Support Policy
 
@@ -47,7 +43,7 @@ Each version of Cluster API for Azure will attempt to support at least two Kuber
 
 **NOTE:** As the versioning for this project is tied to the versioning of Cluster API, future modifications to this policy may be made to more closely align with other providers in the Cluster API ecosystem.
 
----
+------
 
 ## Documentation
 

api/v1alpha3/types.go

@@ -23,22 +23,6 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-// // ResourceSpec defines a generic spec that can used to define Azure resources.
-// TODO: ResourceSpec should be removed once concrete specs have been defined for all Azure resources in use.
-// type ResourceSpec interface{}
-
-// TODO: Write type tests
-
-// AzureResourceReference is a reference to a specific Azure resource by ID
-type AzureResourceReference struct {
-	// ID of resource
-	// +optional
-	ID *string `json:"id,omitempty"`
-	// TODO: Investigate if we should reference resources in other ways
-}
-
-// TODO: Investigate resource filters
-
 // AzureMachineProviderConditionType is a valid value for AzureMachineProviderCondition.Type
 type AzureMachineProviderConditionType string
 
@@ -157,14 +141,6 @@ type SecurityGroup struct {
 	Tags         Tags         `json:"tags,omitempty"`
 }
 
-/*
-// TODO
-// String returns a string representation of the security group.
-func (s *SecurityGroup) String() string {
-	return fmt.Sprintf("id=%s/name=%s", s.ID, s.Name)
-}
-*/
-
 // SecurityGroupProtocol defines the protocol type for a security group rule.
 type SecurityGroupProtocol string
 
@@ -197,45 +173,10 @@ type IngressRule struct {
 	Destination *string `json:"destination,omitempty"`
 }
 
-// TODO
-// String returns a string representation of the ingress rule.
-/*
-func (i *IngressRule) String() string {
-	return fmt.Sprintf("protocol=%s/range=[%d-%d]/description=%s", i.Protocol, i.FromPort, i.ToPort, i.Description)
-}
-*/
-
 // IngressRules is a slice of Azure ingress rules for security groups.
 type IngressRules []*IngressRule
 
-// TODO
-// Difference returns the difference between this slice and the other slice.
-/*
-func (i IngressRules) Difference(o IngressRules) (out IngressRules) {
-	for _, x := range i {
-		found := false
-		for _, y := range o {
-			sort.Strings(x.CidrBlocks)
-			sort.Strings(y.CidrBlocks)
-			sort.Strings(x.SourceSecurityGroupIDs)
-			sort.Strings(y.SourceSecurityGroupIDs)
-			if reflect.DeepEqual(x, y) {
-				found = true
-				break
-			}
-		}
-
-		if !found {
-			out = append(out, x)
-		}
-	}
-
-	return
-}
-*/
-
 // PublicIP defines an Azure public IP address.
-// TODO: Remove once load balancer is implemented.
 type PublicIP struct {
 	ID        string `json:"id,omitempty"`
 	Name      string `json:"name,omitempty"`

cloud/services/subnets/subnets.go

@@ -75,7 +75,6 @@ func (s *Service) Reconcile(ctx context.Context, spec interface{}) error {
 		return errors.New("Invalid Subnet Specification")
 	}
 	if subnet, err := s.Get(ctx, subnetSpec); err == nil {
-		// TODO: add validation on existing subnet
 		// subnet already exists, skip creation
 		if subnetSpec.Role == infrav1.SubnetControlPlane {
 			subnet.DeepCopyInto(s.Scope.ControlPlaneSubnet())

cloud/services/virtualmachineextensions/virtualmachineextensions.go

@@ -78,12 +78,6 @@ func (s *Service) Reconcile(ctx context.Context, spec interface{}) error {
 		return errors.Wrapf(err, "cannot create vm extension")
 	}
 
-	// TODO: review if can remove this code
-	// if *vmExt.ProvisioningState != string(compute.ProvisioningStateSucceeded) {
-	// 	// If the script failed delete it so it can be retried
-	// 	s.Delete(ctx, vmExtSpec)
-	// }
-
 	klog.V(2).Infof("successfully created vm extension %s ", vmExtSpec.Name)
 	return nil
 }

cloud/services/virtualmachines/virtualmachines.go

@@ -275,7 +275,6 @@ func getResourceNameByID(resourceID string) string {
 
 // generateStorageProfile generates a pointer to a compute.StorageProfile which can utilized for VM creation.
 func generateStorageProfile(vmSpec Spec) (*compute.StorageProfile, error) {
-	// TODO: Validate parameters before building storage profile
 	storageProfile := &compute.StorageProfile{
 		OsDisk: &compute.OSDisk{
 			Name:         to.StringPtr(azure.GenerateOSDiskName(vmSpec.Name)),

cloud/services/virtualnetworks/virtualnetworks.go

@@ -89,7 +89,6 @@ func (s *Service) Reconcile(ctx context.Context, spec interface{}) error {
 			s.Scope.V(2).Info("Working on custom vnet", "vnet-id", vnet.ID)
 		}
 		// vnet already exists, cannot update since it's immutable
-		// TODO: ensure tags & other managed vnet attributes
 		vnet.DeepCopyInto(s.Scope.Vnet())
 		return nil
 	}

controllers/azurecluster_controller.go

@@ -123,8 +123,6 @@ func (r *AzureClusterReconciler) reconcileNormal(clusterScope *scope.ClusterScop
 		return reconcile.Result{}, errors.Wrap(err, "failed to reconcile cluster services")
 	}
 
-	// TODO: We may need to use azureCluster.Status.Network.APIServerIP.IPAddress
-	//       instead when we look at configuring private clusters.
 	if azureCluster.Status.Network.APIServerIP.DNSName == "" {
 		clusterScope.Info("Waiting for API server endpoint to exist")
 		return reconcile.Result{RequeueAfter: 15 * time.Second}, nil

controllers/azuremachine_controller.go

@@ -217,24 +217,16 @@ func (r *AzureMachineReconciler) reconcileNormal(ctx context.Context, machineSco
 		return reconcile.Result{}, err
 	}
 
-	// TODO(ncdc): move this validation logic into a validating webhook
-	if errs := r.validateUpdate(&machineScope.AzureMachine.Spec, vm); len(errs) > 0 {
-		agg := kerrors.NewAggregate(errs)
-		r.Recorder.Eventf(machineScope.AzureMachine, corev1.EventTypeWarning, "InvalidUpdate", "Invalid update: %s", agg.Error())
-		return reconcile.Result{}, nil
-	}
-
 	// Make sure Spec.ProviderID is always set.
 	machineScope.SetProviderID(fmt.Sprintf("azure:////%s", vm.ID))
 
-	// Proceed to reconcile the AzureMachine state.
-	machineScope.SetVMState(vm.State)
-
-	// TODO(vincepri): Remove this annotation when clusterctl is no longer relevant.
 	machineScope.SetAnnotation("cluster-api-provider-azure", "true")
 
 	machineScope.SetAddresses(vm.Addresses)
 
+	// Proceed to reconcile the AzureMachine state.
+	machineScope.SetVMState(vm.State)
+
 	switch vm.State {
 	case infrav1.VMStateSucceeded:
 		machineScope.V(2).Info("VM is running", "id", *machineScope.GetVMID())
@@ -306,14 +298,7 @@ func (r *AzureMachineReconciler) reconcileDelete(machineScope *scope.MachineScop
 	return reconcile.Result{}, nil
 }
 
-// validateUpdate checks that no immutable fields have been updated and
-// returns a slice of errors representing attempts to change immutable state.
-func (r *AzureMachineReconciler) validateUpdate(spec *infrav1.AzureMachineSpec, i *infrav1.VM) (errs []error) {
-	// TODO: Add comparison logic for immutable fields
-	return errs
-}
-
-// AzureClusterToAzureMachine is a handler.ToRequestsFunc to be used to enqueue requests for reconciliation
+// AzureClusterToAzureMachines is a handler.ToRequestsFunc to be used to enqueue requests for reconciliation
 // of AzureMachines.
 func (r *AzureMachineReconciler) AzureClusterToAzureMachines(o handler.MapObject) []ctrl.Request {
 	result := []ctrl.Request{}

controllers/azuremachine_reconciler.go

@@ -42,7 +42,6 @@ const (
 )
 
 // azureMachineService are list of services required by cluster actuator, easy to create a fake
-// TODO: We should decide if we want to keep this
 type azureMachineService struct {
 	machineScope          *scope.MachineScope
 	clusterScope          *scope.ClusterScope

hack/tools/go.mod

@@ -24,17 +24,3 @@ require (
 	sigs.k8s.io/kustomize/kustomize/v3 v3.5.4
 	sigs.k8s.io/testing_frameworks v0.1.2
 )
-
-replace (
-	// TODO(vincepri) Remove the following replace directives, needed for golangci-lint until
-	// https://github.com/golangci/golangci-lint/issues/595 is fixed.
-	github.com/go-critic/go-critic v0.0.0-20181204210945-1df300866540 => github.com/go-critic/go-critic v0.0.0-20190526074819-1df300866540
-	github.com/golangci/errcheck v0.0.0-20181003203344-ef45e06d44b6 => github.com/golangci/errcheck v0.0.0-20181223084120-ef45e06d44b6
-	github.com/golangci/go-tools v0.0.0-20180109140146-af6baa5dc196 => github.com/golangci/go-tools v0.0.0-20190318060251-af6baa5dc196
-	github.com/golangci/gofmt v0.0.0-20181105071733-0b8337e80d98 => github.com/golangci/gofmt v0.0.0-20181222123516-0b8337e80d98
-	github.com/golangci/gosec v0.0.0-20180901114220-66fb7fc33547 => github.com/golangci/gosec v0.0.0-20190211064107-66fb7fc33547
-	github.com/golangci/ineffassign v0.0.0-20180808204949-42439a7714cc => github.com/golangci/ineffassign v0.0.0-20190609212857-42439a7714cc
-	github.com/golangci/lint-1 v0.0.0-20180610141402-ee948d087217 => github.com/golangci/lint-1 v0.0.0-20190420132249-ee948d087217
-	github.com/timakin/bodyclose => github.com/golangci/bodyclose v0.0.0-20190714144026-65da19158fa2
-	mvdan.cc/unparam v0.0.0-20190124213536-fbb59629db34 => mvdan.cc/unparam v0.0.0-20190209190245-fbb59629db34
-)

hack/tools/go.sum

@@ -239,8 +239,6 @@ github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5y
 github.com/golang/protobuf v1.3.1/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
 github.com/golang/protobuf v1.3.2 h1:6nsPYzhq5kReh6QImI3k5qWzO4PEbvbIW2cwSfR/6xs=
 github.com/golang/protobuf v1.3.2/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
-github.com/golangci/bodyclose v0.0.0-20190714144026-65da19158fa2 h1:nh/PMhIaxu+h8NOuhOwT2el9Ed08166oitASyNYqQzs=
-github.com/golangci/bodyclose v0.0.0-20190714144026-65da19158fa2/go.mod h1:Qimiffbc6q9tBWlVV6x0P9sat/ao1xEkREYPPj9hphk=
 github.com/golangci/check v0.0.0-20180506172741-cfe4005ccda2 h1:23T5iq8rbUYlhpt5DB4XJkc6BU31uODLD1o1gKvZmD0=
 github.com/golangci/check v0.0.0-20180506172741-cfe4005ccda2/go.mod h1:k9Qvh+8juN+UKMCS/3jFtGICgW8O96FVaZsaxdzDkR4=
 github.com/golangci/dupl v0.0.0-20180902072040-3e9179ac440a h1:w8hkcTqaFpzKqonE9uMCefW1WDie15eSP/4MssdenaM=
@@ -545,6 +543,8 @@ github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81P
 github.com/subosito/gotenv v1.2.0 h1:Slr1R9HxAlEKefgq5jn9U+DnETlIUa6HfgEzj0g5d7s=
 github.com/subosito/gotenv v1.2.0/go.mod h1:N0PQaV/YGNqwC0u51sEeR/aUtSLEXKX9iv69rRypqCw=
 github.com/tidwall/pretty v1.0.0/go.mod h1:XNkn88O1ChpSDQmQeStsy+sBenx6DDtFZJxhVysOjyk=
+github.com/timakin/bodyclose v0.0.0-20190930140734-f7f2e9bca95e h1:RumXZ56IrCj4CL+g1b9OL/oH0QnsF976bC8xQFYUD5Q=
+github.com/timakin/bodyclose v0.0.0-20190930140734-f7f2e9bca95e/go.mod h1:Qimiffbc6q9tBWlVV6x0P9sat/ao1xEkREYPPj9hphk=
 github.com/tmc/grpc-websocket-proxy v0.0.0-20170815181823-89b8d40f7ca8/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U=
 github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U=
 github.com/tommy-muehle/go-mnd v1.1.1 h1:4D0wuPKjOTiK2garzuPGGvm4zZ/wLYDOH8TJSABC7KU=

scripts/ci-conformance.sh

@@ -124,7 +124,6 @@ run_tests() {
 }
 
 get_logs() {
-    # TODO collect more logs https://github.com/kubernetes-sigs/cluster-api-provider-azure/issues/474 
     kubectl logs deploy/capz-controller-manager -n capz-system manager > "${ARTIFACTS}/logs/capz-manager.log" || true
 }
 

test/e2e/generators/capi.go

@@ -49,7 +49,6 @@ func (g *ClusterAPI) releaseYAMLPath() string {
 
 // Manifests return the generated components and any error if there is one.
 func (g *ClusterAPI) Manifests(ctx context.Context) ([]byte, error) {
-	// TODO: this is not very nice
 	if g.GitRef != "" {
 		kustomize := exec.NewCommand(
 			exec.WithCommand("kustomize"),

test/e2e/generators/capz.go

@@ -59,6 +59,5 @@ func expandCredVariables(stdout []byte, creds auth.Creds) []byte {
 	os.Setenv("AZURE_CLIENT_SECRET_B64", base64.StdEncoding.EncodeToString([]byte(creds.ClientSecret)))
 	os.Setenv("AZURE_SUBSCRIPTION_ID_B64", base64.StdEncoding.EncodeToString([]byte(creds.SubscriptionID)))
 	os.Setenv("AZURE_TENANT_ID_B64", base64.StdEncoding.EncodeToString([]byte(creds.TenantID)))
-	// TODO Figure out a better way of doing this
 	return []byte(os.ExpandEnv(string(stdout)))
 }

test/integration/metacluster_test.go

@@ -47,7 +47,4 @@ var _ = Describe("Metacluster", func() {
 	AfterEach(func() {
 		kindCluster.Teardown()
 	})
-
-	// TODO: validate that the controller-manager is deployed and the
-	// types are available
 })