From 25f0ed56af3fb85f8be754995c21b954501770f9 Mon Sep 17 00:00:00 2001 From: Shailesh Pant <7968042+pshail@users.noreply.github.com> Date: Mon, 28 Feb 2022 21:10:36 +0530 Subject: [PATCH] - Fix lint issues - Minor code improvements fixes #250 Signed-off-by: Shailesh Pant <7968042+pshail@users.noreply.github.com> --- agent/cloudinit/cloudinit.go | 2 ++ agent/cloudinit/cmd_runner.go | 2 ++ agent/help_flag_test.go | 2 +- agent/installer/bundle_downloader.go | 9 ++++++++- agent/installer/checks.go | 2 +- agent/installer/cli-dev.go | 1 + agent/installer/installer.go | 2 +- agent/installer/internal/algo/apt_step.go | 1 + agent/main.go | 2 +- .../infrastructure/v1beta1/byocluster_webhook.go | 1 + apis/infrastructure/v1beta1/byohost_types.go | 16 +++++++++++----- apis/infrastructure/v1beta1/byohost_webhook.go | 1 + .../infrastructure/byocluster_controller.go | 2 ++ .../infrastructure/byomachine_controller.go | 8 ++++++-- test/e2e/docker_helper.go | 5 ++++- 15 files changed, 43 insertions(+), 13 deletions(-) diff --git a/agent/cloudinit/cloudinit.go b/agent/cloudinit/cloudinit.go index 5e6647b13..6a327e3ec 100644 --- a/agent/cloudinit/cloudinit.go +++ b/agent/cloudinit/cloudinit.go @@ -14,6 +14,7 @@ import ( "sigs.k8s.io/yaml" ) +// ScriptExecutor bootstrap script executor type ScriptExecutor struct { WriteFilesExecutor IFileWriter RunCmdExecutor ICmdRunner @@ -25,6 +26,7 @@ type bootstrapConfig struct { CommandsToExecute []string `json:"runCmd"` } +// Files details required for files written by bootstrap script type Files struct { Path string `json:"path,"` Encoding string `json:"encoding,omitempty"` diff --git a/agent/cloudinit/cmd_runner.go b/agent/cloudinit/cmd_runner.go index 69ae6e822..80acfe961 100644 --- a/agent/cloudinit/cmd_runner.go +++ b/agent/cloudinit/cmd_runner.go @@ -13,6 +13,8 @@ type ICmdRunner interface { RunCmd(string) error } +// CmdRunner default implementer of ICmdRunner +// TODO reevaluate empty interface/struct type CmdRunner struct { } diff --git a/agent/help_flag_test.go b/agent/help_flag_test.go index c9e559b7a..d2d1720b0 100644 --- a/agent/help_flag_test.go +++ b/agent/help_flag_test.go @@ -41,7 +41,7 @@ var _ = Describe("Help flag for host agent", func() { continue } words := strings.Split(line, " ") - line = (words[0] + " " + words[1]) // checking the first two words + line = words[0] + " " + words[1] // checking the first two words // Any option not belongs to expectedOptions is not allowed. Expect(strings.TrimSpace(line)).To(BeElementOf(expectedOptions)) } diff --git a/agent/installer/bundle_downloader.go b/agent/installer/bundle_downloader.go index 6355f2510..36315515d 100644 --- a/agent/installer/bundle_downloader.go +++ b/agent/installer/bundle_downloader.go @@ -16,6 +16,7 @@ import ( ) var ( + // DownloadPathPermissions file mode permissions for download path DownloadPathPermissions fs.FileMode = 0777 ) @@ -53,7 +54,13 @@ func (bd *bundleDownloader) DownloadFromRepo( downloadPathWithRepo := bd.getBundlePathWithRepo() err := ensureDirExist(downloadPathWithRepo) - defer os.Remove(downloadPathWithRepo) + defer func(name string) { + err = os.Remove(name) + if err != nil { + bd.logger.Error(err, "Failed to remove directory", "path", name) + } + }(downloadPathWithRepo) + if err != nil { return err } diff --git a/agent/installer/checks.go b/agent/installer/checks.go index 56af47278..063b194d7 100644 --- a/agent/installer/checks.go +++ b/agent/installer/checks.go @@ -13,7 +13,7 @@ import ( func checkPreRequsitePackages() error { if runtime.GOOS == "linux" { - unavailablePackages := []string{} + unavailablePackages := make([]string, 0) execr := utilsexec.New() for _, pkgName := range preRequisitePackages { _, err := execr.LookPath(pkgName) diff --git a/agent/installer/cli-dev.go b/agent/installer/cli-dev.go index 70685ae57..2cccab44a 100644 --- a/agent/installer/cli-dev.go +++ b/agent/installer/cli-dev.go @@ -35,6 +35,7 @@ var ( klogger logr.Logger ) +// Main entry point for the installer dev/test CLI func Main() { klogger = klogr.New() diff --git a/agent/installer/installer.go b/agent/installer/installer.go index 1ef03ca48..7ed9848ed 100644 --- a/agent/installer/installer.go +++ b/agent/installer/installer.go @@ -149,7 +149,7 @@ func (i *installer) Install(bundleRepo, k8sVer, tag string) error { return nil } -// Uninstal uninstalls the specified k8s version on the current OS +// Uninstall uninstalls the specified k8s version on the current OS func (i *installer) Uninstall(bundleRepo, k8sVer, tag string) error { i.setBundleRepo(bundleRepo) algoInst, err := i.getAlgoInstallerWithBundle(k8sVer, tag) diff --git a/agent/installer/internal/algo/apt_step.go b/agent/installer/internal/algo/apt_step.go index ce27aa984..365ab7250 100755 --- a/agent/installer/internal/algo/apt_step.go +++ b/agent/installer/internal/algo/apt_step.go @@ -9,6 +9,7 @@ import ( "strings" ) +// NewAptStep returns a new step to install apt package func NewAptStep(k *BaseK8sInstaller, aptPkg string) Step { return NewAptStepEx(k, aptPkg, false) } diff --git a/agent/main.go b/agent/main.go index a3839bfe1..0cb67585e 100644 --- a/agent/main.go +++ b/agent/main.go @@ -10,7 +10,7 @@ import ( "os" "strings" - pflag "github.com/spf13/pflag" + "github.com/spf13/pflag" "github.com/vmware-tanzu/cluster-api-provider-bringyourownhost/agent/cloudinit" "github.com/vmware-tanzu/cluster-api-provider-bringyourownhost/agent/installer" "github.com/vmware-tanzu/cluster-api-provider-bringyourownhost/agent/reconciler" diff --git a/apis/infrastructure/v1beta1/byocluster_webhook.go b/apis/infrastructure/v1beta1/byocluster_webhook.go index f6b40fda8..1d7279bf4 100644 --- a/apis/infrastructure/v1beta1/byocluster_webhook.go +++ b/apis/infrastructure/v1beta1/byocluster_webhook.go @@ -17,6 +17,7 @@ import ( // log is for logging in this package. var byoclusterlog = logf.Log.WithName("byocluster-resource") +// SetupWebhookWithManager sets up the webhook for the byocluster resource func (byoCluster *ByoCluster) SetupWebhookWithManager(mgr ctrl.Manager) error { return ctrl.NewWebhookManagedBy(mgr). For(byoCluster). diff --git a/apis/infrastructure/v1beta1/byohost_types.go b/apis/infrastructure/v1beta1/byohost_types.go index f5fac481c..7e2c03202 100644 --- a/apis/infrastructure/v1beta1/byohost_types.go +++ b/apis/infrastructure/v1beta1/byohost_types.go @@ -10,12 +10,18 @@ import ( ) const ( - HostCleanupAnnotation = "byoh.infrastructure.cluster.x-k8s.io/unregistering" - EndPointIPAnnotation = "byoh.infrastructure.cluster.x-k8s.io/endpointip" - K8sVersionAnnotation = "byoh.infrastructure.cluster.x-k8s.io/k8sversion" - AttachedByoMachineLabel = "byoh.infrastructure.cluster.x-k8s.io/byomachine-name" + // HostCleanupAnnotation annotation used to mark a host for cleanup + HostCleanupAnnotation = "byoh.infrastructure.cluster.x-k8s.io/unregistering" + // EndPointIPAnnotation annotation used to store the IP address of the endpoint + EndPointIPAnnotation = "byoh.infrastructure.cluster.x-k8s.io/endpointip" + // K8sVersionAnnotation annotation used to store the k8s version + K8sVersionAnnotation = "byoh.infrastructure.cluster.x-k8s.io/k8sversion" + // AttachedByoMachineLabel label used to mark a node name attached to a byo host + AttachedByoMachineLabel = "byoh.infrastructure.cluster.x-k8s.io/byomachine-name" + // BundleLookupBaseRegistryAnnotation annotation used to store the base registry for the bundle lookup BundleLookupBaseRegistryAnnotation = "byoh.infrastructure.cluster.x-k8s.io/bundle-registry" - BundleLookupTagAnnotation = "byoh.infrastructure.cluster.x-k8s.io/bundle-tag" + // BundleLookupTagAnnotation annotation used to store the bundle tag + BundleLookupTagAnnotation = "byoh.infrastructure.cluster.x-k8s.io/bundle-tag" ) // ByoHostSpec defines the desired state of ByoHost diff --git a/apis/infrastructure/v1beta1/byohost_webhook.go b/apis/infrastructure/v1beta1/byohost_webhook.go index 88f0f197f..58c2243e4 100644 --- a/apis/infrastructure/v1beta1/byohost_webhook.go +++ b/apis/infrastructure/v1beta1/byohost_webhook.go @@ -17,6 +17,7 @@ import ( // log is for logging in this package var byohostlog = logf.Log.WithName("byohost-resource") +// SetupWebhookWithManager sets up the webhook for the byohost resource func (byoHost *ByoHost) SetupWebhookWithManager(mgr ctrl.Manager) error { return ctrl.NewWebhookManagedBy(mgr). For(byoHost). diff --git a/controllers/infrastructure/byocluster_controller.go b/controllers/infrastructure/byocluster_controller.go index 4d5335777..950036a00 100644 --- a/controllers/infrastructure/byocluster_controller.go +++ b/controllers/infrastructure/byocluster_controller.go @@ -29,6 +29,7 @@ import ( ) var ( + // DefaultAPIEndpointPort default port for the API endpoint DefaultAPIEndpointPort = 6443 clusterControlledType = &infrav1.ByoCluster{} clusterControlledTypeName = reflect.TypeOf(clusterControlledType).Elem().Name() @@ -46,6 +47,7 @@ type ByoClusterReconciler struct { //+kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=byoclusters/finalizers,verbs=update // +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=clusters;clusters/status,verbs=get;list;watch +// Reconcile handles the byo cluster reconciliations func (r *ByoClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Result, reterr error) { logger := log.FromContext(ctx) diff --git a/controllers/infrastructure/byomachine_controller.go b/controllers/infrastructure/byomachine_controller.go index c638e0360..9a3513955 100644 --- a/controllers/infrastructure/byomachine_controller.go +++ b/controllers/infrastructure/byomachine_controller.go @@ -41,9 +41,12 @@ import ( ) const ( - ProviderIDPrefix = "byoh://" + // ProviderIDPrefix prefix for provider id + ProviderIDPrefix = "byoh://" + // ProviderIDSuffixLength length of provider id suffix ProviderIDSuffixLength = 6 - RequeueForbyohost = 10 * time.Second + // RequeueForbyohost requeue delay for byoh host + RequeueForbyohost = 10 * time.Second ) // ByoMachineReconciler reconciles a ByoMachine object @@ -177,6 +180,7 @@ func (r *ByoMachineReconciler) Reconcile(ctx context.Context, req ctrl.Request) return r.reconcileNormal(ctx, machineScope) } +// FetchAttachedByoHost fetches BYOHost attached to this machine func (r *ByoMachineReconciler) FetchAttachedByoHost(ctx context.Context, byomachineName, byomachineNamespace string) (*infrav1.ByoHost, error) { logger := log.FromContext(ctx) logger.Info("Fetching an attached ByoHost") diff --git a/test/e2e/docker_helper.go b/test/e2e/docker_helper.go index 6e15edea5..ab3e96fc5 100644 --- a/test/e2e/docker_helper.go +++ b/test/e2e/docker_helper.go @@ -264,5 +264,8 @@ func setControlPlaneIP(ctx context.Context, dockerClient *client.Client) { // can safely use this IP for the ControlPlaneEndpoint ipOctets[3] = "151" ip := strings.Join(ipOctets, ".") - os.Setenv("CONTROL_PLANE_ENDPOINT_IP", ip) + err := os.Setenv("CONTROL_PLANE_ENDPOINT_IP", ip) + if err != nil { + Expect(err).NotTo(HaveOccurred()) + } }