Skip to content

Commit

Permalink
- Fix lint issues
Browse files Browse the repository at this point in the history
 - Minor code improvements
 - Unexported locally used variables
fixes vmware-tanzu#250

-- Rebased --
 21st March

Signed-off-by: Shailesh Pant <[email protected]>
  • Loading branch information
pshail committed Mar 21, 2022
1 parent 3e0b04f commit d7a9259
Show file tree
Hide file tree
Showing 19 changed files with 73 additions and 22 deletions.
2 changes: 2 additions & 0 deletions agent/cloudinit/cloudinit.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"sigs.k8s.io/yaml"
)

// ScriptExecutor bootstrap script executor
type ScriptExecutor struct {
WriteFilesExecutor IFileWriter
RunCmdExecutor ICmdRunner
Expand All @@ -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"`
Expand Down
2 changes: 2 additions & 0 deletions agent/cloudinit/cmd_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ type ICmdRunner interface {
RunCmd(string) error
}

// CmdRunner default implementer of ICmdRunner
// TODO reevaluate empty interface/struct
type CmdRunner struct {
}

Expand Down
1 change: 1 addition & 0 deletions agent/cloudinit/file_writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ type IFileWriter interface {
WriteToFile(*Files) error
}

// FileWriter default implementation of IFileWriter
type FileWriter struct {
}

Expand Down
2 changes: 1 addition & 1 deletion agent/help_flag_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand Down
9 changes: 8 additions & 1 deletion agent/installer/bundle_downloader.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
)

var (
// DownloadPathPermissions file mode permissions for download path
DownloadPathPermissions fs.FileMode = 0777
)

Expand Down Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion agent/installer/checks.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions agent/installer/cli-dev.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ var (
klogger logr.Logger
)

// Main entry point for the installer dev/test CLI
func Main() {
klogger = klogr.New()

Expand Down
2 changes: 1 addition & 1 deletion agent/installer/installer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions agent/installer/internal/algo/apt_step.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion agent/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
5 changes: 4 additions & 1 deletion agent/reconciler/host_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ type IK8sInstaller interface {
Uninstall(string, string, string) error
}

// HostReconciler encapsulates the data/logic needed to reconcile a ByoHost
type HostReconciler struct {
Client client.Client
CmdRunner cloudinit.ICmdRunner
Expand All @@ -46,7 +47,8 @@ type HostReconciler struct {

const (
bootstrapSentinelFile = "/run/cluster-api/bootstrap-success.complete"
KubeadmResetCommand = "kubeadm reset --force"
// KubeadmResetCommand is the command to run to force reset/remove nodes' local file system of the files created by kubeadm
KubeadmResetCommand = "kubeadm reset --force"
)

// Reconcile handles events for the ByoHost that is registered by this agent process
Expand Down Expand Up @@ -161,6 +163,7 @@ func (r *HostReconciler) getBootstrapScript(ctx context.Context, dataSecretName,
return bootstrapSecret, nil
}

// SetupWithManager sets up the controller with the manager
func (r *HostReconciler) SetupWithManager(ctx context.Context, mgr manager.Manager) error {
return ctrl.NewControllerManagedBy(mgr).
For(&infrastructurev1beta1.ByoHost{}).
Expand Down
5 changes: 5 additions & 0 deletions agent/registration/host_registrar.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,16 @@ import (
)

var (
// LocalHostRegistrar is a HostRegistrar that registers the local host.
LocalHostRegistrar *HostRegistrar
)

// HostInfo contains information about the host network interface.
type HostInfo struct {
DefaultNetworkInterfaceName string
}

// HostRegistrar used to register a host.
type HostRegistrar struct {
K8sClient client.Client
ByoHostInfo HostInfo
Expand Down Expand Up @@ -67,6 +70,7 @@ func (hr *HostRegistrar) Register(hostName, namespace string, hostLabels map[str
return hr.UpdateNetwork(ctx, byoHost)
}

// UpdateNetwork updates the network interface status for the host
func (hr *HostRegistrar) UpdateNetwork(ctx context.Context, byoHost *infrastructurev1beta1.ByoHost) error {
klog.Info("Add Network Info")
helper, err := patch.NewHelper(byoHost, hr.K8sClient)
Expand All @@ -79,6 +83,7 @@ func (hr *HostRegistrar) UpdateNetwork(ctx context.Context, byoHost *infrastruct
return helper.Patch(ctx, byoHost)
}

// GetNetworkStatus returns the network interface(s) status for the host
func (hr *HostRegistrar) GetNetworkStatus() []infrastructurev1beta1.NetworkStatus {
Network := make([]infrastructurev1beta1.NetworkStatus, 0)

Expand Down
1 change: 1 addition & 0 deletions apis/infrastructure/v1beta1/byocluster_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
16 changes: 11 additions & 5 deletions apis/infrastructure/v1beta1/byohost_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions apis/infrastructure/v1beta1/byohost_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
2 changes: 2 additions & 0 deletions controllers/infrastructure/byocluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
)

var (
// DefaultAPIEndpointPort default port for the API endpoint
DefaultAPIEndpointPort = 6443
clusterControlledType = &infrav1.ByoCluster{}
clusterControlledTypeName = reflect.TypeOf(clusterControlledType).Elem().Name()
Expand All @@ -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)

Expand Down
8 changes: 6 additions & 2 deletions controllers/infrastructure/byomachine_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")
Expand Down
18 changes: 12 additions & 6 deletions test/e2e/docker_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ import (
)

const (
KindImage = "byoh/node:v1.22.3"
TempKubeconfigPath = "/tmp/mgmt.conf"
kindImage = "byoh/node:v1.22.3"
tempKubeconfigPath = "/tmp/mgmt.conf"
)

type cpConfig struct {
Expand All @@ -37,6 +37,7 @@ type cpConfig struct {
container string
}

// ByoHostRunner runs bring-you-own-host cluster in docker
type ByoHostRunner struct {
Context context.Context
clusterConName string
Expand Down Expand Up @@ -160,7 +161,7 @@ func (r *ByoHostRunner) createDockerContainer() (container.ContainerCreateCreate

return r.DockerClient.ContainerCreate(r.Context,
&container.Config{Hostname: r.ByoHostName,
Image: KindImage,
Image: kindImage,
},
&container.HostConfig{Privileged: true,
SecurityOpt: []string{"seccomp=unconfined"},
Expand Down Expand Up @@ -200,14 +201,15 @@ func (r *ByoHostRunner) copyKubeconfig(config cpConfig, listopt types.ContainerL
re := regexp.MustCompile("server:.*")
kubeconfig = re.ReplaceAll(kubeconfig, []byte("server: https://"+profile.NetworkSettings.Networks[r.NetworkInterface].IPAddress+":6443"))
}
Expect(os.WriteFile(TempKubeconfigPath, kubeconfig, 0644)).NotTo(HaveOccurred()) // nolint: gosec,gomnd
Expect(os.WriteFile(tempKubeconfigPath, kubeconfig, 0644)).NotTo(HaveOccurred()) // nolint: gosec,gomnd

config.sourcePath = TempKubeconfigPath
config.sourcePath = tempKubeconfigPath
config.destPath = r.CommandArgs["--kubeconfig"]
err := copyToContainer(r.Context, r.DockerClient, config)
return err
}

// SetupByoDockerHost sets up the byohost docker container
func (r *ByoHostRunner) SetupByoDockerHost() (*container.ContainerCreateCreatedBody, error) {
var byohost container.ContainerCreateCreatedBody
var err error
Expand All @@ -231,6 +233,7 @@ func (r *ByoHostRunner) SetupByoDockerHost() (*container.ContainerCreateCreatedB
return &byohost, err
}

// ExecByoDockerHost runs the exec command in the byohost docker container
func (r *ByoHostRunner) ExecByoDockerHost(byohost *container.ContainerCreateCreatedBody) (types.HijackedResponse, string, error) {
var cmdArgs []string
cmdArgs = append(cmdArgs, "./agent")
Expand Down Expand Up @@ -264,5 +267,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())
}
}
15 changes: 12 additions & 3 deletions test/e2e/e2e_debug_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,15 @@ import (
)

const (
DefaultFileMode fs.FileMode = 0777
ReadByohControllerManagerLogShellFile string = "/tmp/read-byoh-controller-manager-log.sh"
ReadAllPodsShellFile string = "/tmp/read-all-pods.sh"
// DefaultFileMode the default file mode of files created for tests
DefaultFileMode fs.FileMode = 0777
// ReadByohControllerManagerLogShellFile location of script to read the controller manager log
ReadByohControllerManagerLogShellFile string = "/tmp/read-byoh-controller-manager-log.sh"
// ReadAllPodsShellFile location of script to read all pods logs
ReadAllPodsShellFile string = "/tmp/read-all-pods.sh"
)

// WriteDockerLog redirects the docker logs to the given file
func WriteDockerLog(output types.HijackedResponse, outputFile string) *os.File {
s := make(chan string)
e := make(chan error)
Expand Down Expand Up @@ -62,11 +66,13 @@ func WriteDockerLog(output types.HijackedResponse, outputFile string) *os.File {
return f
}

// Showf prints formatted string to stdout
func Showf(format string, a ...interface{}) {
fmt.Printf(format, a...)
fmt.Printf("\n")
}

// ShowFileContent prints to stdout the content of the given file
func ShowFileContent(fileName string) {
content, err := os.ReadFile(fileName)
if err != nil {
Expand All @@ -79,6 +85,7 @@ func ShowFileContent(fileName string) {
Showf("######################End: Content of %s##################", fileName)
}

// ExecuteShellScript executes a given shell script file location
func ExecuteShellScript(shellFileName string) {
cmd := exec.Command("/bin/sh", "-x", shellFileName)
output, err := cmd.Output()
Expand All @@ -91,6 +98,7 @@ func ExecuteShellScript(shellFileName string) {
Showf("######################End: execute result of %s##################", shellFileName)
}

// WriteShellScript writes shell script contents/commands to the given file location
func WriteShellScript(shellFileName string, shellFileContent []string) {
f, err := os.OpenFile(shellFileName, os.O_APPEND|os.O_WRONLY|os.O_CREATE, DefaultFileMode)
if err != nil {
Expand All @@ -117,6 +125,7 @@ func WriteShellScript(shellFileName string, shellFileContent []string) {
}
}

// ShowInfo shows all the pods status, agent logs, and controller manager logs
func ShowInfo(allAgentLogFiles []string) {
// show swap status
// showFileContent("/proc/swaps")
Expand Down

0 comments on commit d7a9259

Please sign in to comment.