Merge pull request #379 from jacobweinstock/file-reorg

Reorganize and rename packages for improved cognitive load

## Description

<!--- Please describe what this PR is going to change -->

Packages in Go should generally be [singular in name](https://rakyll.org/style-packages/#no-plurals). So the pkg `controllers` was renamed to `controller`. The 2 controllers, `tinkerbellcluster` and `tinkerbellmachine` were put into their own packages. This was done to help improve the cognitive load for understanding and maintaining this repo.

Additional files in the machine package were created to also help with the cognitive load of that controller.

> [!NOTE] 
> No functionality has been changed.

## Why is this needed

<!--- Link to issue you have raised -->

Fixes: #

## How Has This Been Tested?
<!--- Please describe in detail how you tested your changes. -->
<!--- Include details of your testing environment, and the tests you ran to -->
<!--- see how your change affects other areas of the code, etc. -->


## How are existing users impacted? What migration steps/scripts do we need?

<!--- Fixes a bug, unblocks installation, removes a component of the stack etc -->
<!--- Requires a DB migration script, etc. -->


## Checklist:

I have:

- [ ] updated the documentation and/or roadmap (if required)
- [ ] added unit or e2e tests
- [ ] provided instructions on how to upgrade

.golangci.yml

@@ -84,7 +84,7 @@ issues:
       source: "^// \\+kubebuilder:"
     - linters:
         - lll
-      path: "controllers/(.+)_test.go"
+      path: "controller/*/(.+)_test.go"
   exclude-files:
     - "zz_generated.*\\.go$"
     - ".*conversion.*\\.go$"

Makefile

@@ -217,7 +217,7 @@ generate-manifests: tools ## Generate manifests e.g. CRD, RBAC etc.
 		output:webhook:dir=$(WEBHOOK_ROOT) \
 		webhook
 	$(CONTROLLER_GEN) \
-		paths=./controllers/... \
+		paths=./controller/... \
 		output:rbac:dir=$(RBAC_ROOT) \
 		rbac:roleName=manager-role
 

config/default/manager_image_patch.yaml

@@ -8,5 +8,5 @@ spec:
     spec:
       containers:
       # Change the value of image field below to your controller image URL
-      - image: ghcr.io/tinkerbell/cluster-api-provider-tinkerbell:dev
+      - image: ghcr.io/tinkerbell/cluster-api-provider-tinkerbell-amd64:dev
         name: manager

config/default/manager_pull_policy.yaml

@@ -8,4 +8,4 @@ spec:
     spec:
       containers:
       - name: manager
-        imagePullPolicy: IfNotPresent
+        imagePullPolicy: Always

controllers/tinkerbellcluster_controller.go → controller/cluster/tinkerbellcluster.go

@@ -14,8 +14,8 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-// Package controllers contains Cluster API controllers for Tinkerbell.
-package controllers
+// Package cluster contains Cluster API controller for the TinkerbellCluster CR.
+package cluster
 
 import (
 	"context"
@@ -35,11 +35,32 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/controller"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 
-	tinkv1 "github.com/tinkerbell/tink/api/v1alpha1"
-
 	infrastructurev1 "github.com/tinkerbell/cluster-api-provider-tinkerbell/api/v1beta1"
 )
 
+const (
+	// ClusterNameLabel is used to mark Hardware as assigned controlplane machine.
+	ClusterNameLabel = "v1alpha1.tinkerbell.org/clusterName"
+
+	// ClusterNamespaceLabel is used to mark in which Namespace hardware is used.
+	ClusterNamespaceLabel = "v1alpha1.tinkerbell.org/clusterNamespace"
+
+	// KubernetesAPIPort is a port used by Tinkerbell clusters for Kubernetes API.
+	KubernetesAPIPort = 6443
+)
+
+var (
+	// ErrClusterNotReady is returned when trying to reconcile prior to the Cluster resource being ready.
+	ErrClusterNotReady = fmt.Errorf("cluster resource not ready")
+	// ErrControlPlaneEndpointNotSet is returned when trying to reconcile when the ControlPlane Endpoint is not defined.
+	ErrControlPlaneEndpointNotSet = fmt.Errorf("controlplane endpoint is not set")
+	// ErrConfigurationNil is the error returned when TinkerbellMachineReconciler or TinkerbellClusterReconciler is nil.
+	ErrConfigurationNil = fmt.Errorf("configuration is nil")
+	// ErrMissingClient is the error returned when TinkerbellMachineReconciler or TinkerbellClusterReconciler do
+	// not have a Client configured.
+	ErrMissingClient = fmt.Errorf("client is nil")
+)
+
 // TinkerbellClusterReconciler implements Reconciler interface.
 type TinkerbellClusterReconciler struct {
 	client.Client
@@ -126,69 +147,6 @@ type clusterReconcileContext struct {
 	namespacedName    types.NamespacedName
 }
 
-const (
-	// HardwareOwnerNameLabel is a label set by either CAPT controllers or Tinkerbell controller to indicate
-	// that given hardware takes part of at least one workflow.
-	HardwareOwnerNameLabel = "v1alpha1.tinkerbell.org/ownerName"
-
-	// HardwareOwnerNamespaceLabel is a label set by either CAPT controllers or Tinkerbell controller to indicate
-	// that given hardware takes part of at least one workflow.
-	HardwareOwnerNamespaceLabel = "v1alpha1.tinkerbell.org/ownerNamespace"
-
-	// ClusterNameLabel is used to mark Hardware as assigned controlplane machine.
-	ClusterNameLabel = "v1alpha1.tinkerbell.org/clusterName"
-
-	// ClusterNamespaceLabel is used to mark in which Namespace hardware is used.
-	ClusterNamespaceLabel = "v1alpha1.tinkerbell.org/clusterNamespace"
-
-	// KubernetesAPIPort is a port used by Tinkerbell clusters for Kubernetes API.
-	KubernetesAPIPort = 6443
-)
-
-var (
-	// ErrNoHardwareAvailable is the error returned when there is no hardware available for provisioning.
-	ErrNoHardwareAvailable = fmt.Errorf("no hardware available")
-	// ErrHardwareIsNil is the error returned when the given hardware resource is nil.
-	ErrHardwareIsNil = fmt.Errorf("given Hardware object is nil")
-	// ErrHardwareMissingInterfaces is the error returned when the referenced hardware does not have any
-	// network interfaces defined.
-	ErrHardwareMissingInterfaces = fmt.Errorf("hardware has no interfaces defined")
-	// ErrHardwareFirstInterfaceNotDHCP is the error returned when the referenced hardware does not have it's
-	// first network interface configured for DHCP.
-	ErrHardwareFirstInterfaceNotDHCP = fmt.Errorf("hardware's first interface has no DHCP address defined")
-	// ErrHardwareFirstInterfaceDHCPMissingIP is the error returned when the referenced hardware does not have a
-	// DHCP IP address assigned for it's first interface.
-	ErrHardwareFirstInterfaceDHCPMissingIP = fmt.Errorf("hardware's first interface has no DHCP IP address defined")
-	// ErrClusterNotReady is returned when trying to reconcile prior to the Cluster resource being ready.
-	ErrClusterNotReady = fmt.Errorf("cluster resource not ready")
-	// ErrControlPlaneEndpointNotSet is returned when trying to reconcile when the ControlPlane Endpoint is not defined.
-	ErrControlPlaneEndpointNotSet = fmt.Errorf("controlplane endpoint is not set")
-)
-
-func hardwareIP(hardware *tinkv1.Hardware) (string, error) {
-	if hardware == nil {
-		return "", ErrHardwareIsNil
-	}
-
-	if len(hardware.Spec.Interfaces) == 0 {
-		return "", ErrHardwareMissingInterfaces
-	}
-
-	if hardware.Spec.Interfaces[0].DHCP == nil {
-		return "", ErrHardwareFirstInterfaceNotDHCP
-	}
-
-	if hardware.Spec.Interfaces[0].DHCP.IP == nil {
-		return "", ErrHardwareFirstInterfaceDHCPMissingIP
-	}
-
-	if hardware.Spec.Interfaces[0].DHCP.IP.Address == "" {
-		return "", ErrHardwareFirstInterfaceDHCPMissingIP
-	}
-
-	return hardware.Spec.Interfaces[0].DHCP.IP.Address, nil
-}
-
 func (crc *clusterReconcileContext) controlPlaneEndpoint() (clusterv1.APIEndpoint, error) {
 	switch {
 	case crc.tinkerbellCluster.Spec.ControlPlaneEndpoint.IsValid():

controllers/tinkerbellcluster_controller_test.go → controller/cluster/tinkerbellcluster_test.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package controllers_test
+package cluster_test
 
 import (
 	"context"
@@ -23,8 +23,10 @@ import (
 	"github.com/google/uuid"
 	. "github.com/onsi/gomega" //nolint:revive // one day we will remove gomega
 	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/utils/ptr"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -33,7 +35,7 @@ import (
 	tinkv1 "github.com/tinkerbell/tink/api/v1alpha1"
 
 	infrastructurev1 "github.com/tinkerbell/cluster-api-provider-tinkerbell/api/v1beta1"
-	"github.com/tinkerbell/cluster-api-provider-tinkerbell/controllers"
+	"github.com/tinkerbell/cluster-api-provider-tinkerbell/controller/cluster"
 )
 
 //nolint:unparam
@@ -60,7 +62,7 @@ func Test_Cluster_reconciliation_when_controlplane_endpoint_not_set(t *testing.T
 	client := kubernetesClientWithObjects(t, objects)
 
 	_, err := reconcileClusterWithClient(client, clusterName, clusterNamespace)
-	g.Expect(err).To(MatchError(controllers.ErrControlPlaneEndpointNotSet))
+	g.Expect(err).To(MatchError(cluster.ErrControlPlaneEndpointNotSet))
 }
 
 func Test_Cluster_reconciliation_when_controlplane_endpoint_set_on_cluster(t *testing.T) {
@@ -100,6 +102,58 @@ func Test_Cluster_reconciliation_when_controlplane_endpoint_set_on_cluster(t *te
 	g.Expect(updatedTinkerbellCluster.Status.Ready).To(BeTrue(), "Expected infrastructure to be ready")
 }
 
+type testOptions struct {
+	// Labels allow providing labels for the machine
+	Labels           map[string]string
+	HardwareAffinity *infrastructurev1.HardwareAffinity
+}
+
+func validHardware(name, uuid, ip string, options ...testOptions) *tinkv1.Hardware {
+	hw := &tinkv1.Hardware{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      name,
+			Namespace: clusterNamespace,
+			UID:       types.UID(uuid),
+		},
+		Spec: tinkv1.HardwareSpec{
+			Disks: []tinkv1.Disk{
+				{
+					Device: "/dev/sda",
+				},
+			},
+			Interfaces: []tinkv1.Interface{
+				{
+					DHCP: &tinkv1.DHCP{
+						IP: &tinkv1.IP{
+							Address: ip,
+						},
+					},
+					Netboot: &tinkv1.Netboot{
+						AllowPXE: ptr.To(true),
+					},
+				},
+			},
+			Metadata: &tinkv1.HardwareMetadata{
+				Instance: &tinkv1.MetadataInstance{
+					ID: ip,
+				},
+			},
+		},
+	}
+
+	for _, o := range options {
+		for k, v := range o.Labels {
+			if hw.Labels == nil {
+				hw.Labels = map[string]string{}
+			}
+
+			hw.Labels[k] = v
+		}
+	}
+
+	return hw
+}
+
 func Test_Cluster_reconciliation_when_controlplane_endpoint_set_on_tinkerbellCluster(t *testing.T) {
 	t.Parallel()
 	g := NewWithT(t)
@@ -171,7 +225,7 @@ func clusterReconciliationFailsWhenReconcilerHasNoClientSet(t *testing.T) {
 	t.Parallel()
 	g := NewWithT(t)
 
-	clusterController := &controllers.TinkerbellClusterReconciler{}
+	clusterController := &cluster.TinkerbellClusterReconciler{}
 
 	request := ctrl.Request{
 		NamespacedName: types.NamespacedName{
@@ -181,7 +235,7 @@ func clusterReconciliationFailsWhenReconcilerHasNoClientSet(t *testing.T) {
 	}
 
 	_, err := clusterController.Reconcile(context.TODO(), request)
-	g.Expect(err).To(MatchError(controllers.ErrMissingClient))
+	g.Expect(err).To(MatchError(cluster.ErrMissingClient))
 }
 
 func kubernetesClientWithObjects(t *testing.T, objects []runtime.Object) client.Client {
@@ -205,7 +259,7 @@ func kubernetesClientWithObjects(t *testing.T, objects []runtime.Object) client.
 
 //nolint:unparam
 func reconcileClusterWithClient(client client.Client, name, namespace string) (ctrl.Result, error) {
-	clusterController := &controllers.TinkerbellClusterReconciler{
+	clusterController := &cluster.TinkerbellClusterReconciler{
 		Client: client,
 	}
 
@@ -235,6 +289,51 @@ const (
 	hardwareName     = "myHardwareName"
 )
 
+//nolint:unparam
+func validCluster(name, namespace string) *clusterv1.Cluster {
+	return &clusterv1.Cluster{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      name,
+			Namespace: namespace,
+		},
+		Spec: clusterv1.ClusterSpec{
+			InfrastructureRef: &corev1.ObjectReference{
+				Name: name,
+			},
+		},
+	}
+}
+
+func validTinkerbellCluster(name, namespace string) *infrastructurev1.TinkerbellCluster {
+	tinkCluster := &infrastructurev1.TinkerbellCluster{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:       name,
+			Namespace:  namespace,
+			Finalizers: []string{infrastructurev1.ClusterFinalizer},
+			OwnerReferences: []metav1.OwnerReference{
+				{
+					APIVersion: "cluster.x-k8s.io/v1beta1",
+					Kind:       "Cluster",
+					Name:       name,
+				},
+			},
+		},
+		Spec: infrastructurev1.TinkerbellClusterSpec{
+			ControlPlaneEndpoint: clusterv1.APIEndpoint{
+				Host: hardwareIP,
+				Port: 6443,
+			},
+		},
+		Status: infrastructurev1.TinkerbellClusterStatus{
+			Ready: true,
+		},
+	}
+
+	tinkCluster.Default()
+
+	return tinkCluster
+}
+
 func clusterReconciliationIsNotRequeuedWhenClusterHasNoOwnerSet(t *testing.T) {
 	t.Parallel()
 	g := NewWithT(t)