diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000000..63fdd13f6b --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,109 @@ +--- +run: + concurrency: 6 + deadline: 5m + skip-files: + - ".*_test\\.go" + - "lib/.*" +linters: + disable-all: true + enable: + - deadcode + - depguard + #- dupl + #- gochecknoinits + - goconst + - gocritic + - gofmt + - goimports + - golint + - gosec + - govet + - ineffassign + #- interfacer + #- maligned + - misspell + #- prealloc + #- scopelint + - staticcheck + - structcheck + - typecheck + - unconvert + - unparam + - varcheck + #- errcheck + #- gochecknoglobals + - gocyclo + #- lll + #- nakedret +linters-settings: + gocritic: + enabled-checks: + # Diagnostic + - argOrder + - badCond + - caseOrder + - codegenComment + - commentedOutCode + - deprecatedComment + - dupArg + - dupBranchBody + - dupCase + - dupSubExpr + - exitAfterDefer + - flagDeref + - flagName + - nilValReturn + - offBy1 + - weakCond + #- appendAssign + - octalLiteral + - sloppyReassign + + # Performance + - equalFold + #- hugeParam + - indexAlloc + - rangeExprCopy + #- rangeValCopy + - appendCombine + + # Style + - assignOp + - boolExprSimplify + - captLocal + - commentFormatting + - commentedOutImport + - defaultCaseOrder + - docStub + - elseif + - emptyFallthrough + - emptyStringTest + - hexLiteral + #- ifElseChain + - methodExprCall + - regexpMust + - singleCaseSwitch + - sloppyLen + - stringXbytes + - switchTrue + - typeAssertChain + - typeSwitchVar + - underef + - unlabelStmt + - unlambda + - unslice + - valSwap + - yodaStyleExpr + - wrapperFunc + + # Opinionated + - initClause + - nestingReduce + - ptrToRefParam + - typeUnparen + - unnecessaryBlock + #- builtinShadow + #- importShadow + - paramTypeCombine + #- unnamedResult diff --git a/Makefile b/Makefile index d8d22fc431..74cfe6a612 100644 --- a/Makefile +++ b/Makefile @@ -50,6 +50,8 @@ update: # make verify verify: @which go-bindata 2> /dev/null >&1 || { echo "go-bindata must be installed to verify generated code"; exit 1; } + @which golangci-lint 2> /dev/null >&1 || { echo "golangci-lint must be installed to lint code"; exit 1; } + golangci-lint run hack/verify-codegen.sh hack/verify-generated-bindata.sh diff --git a/cmd/machine-config-operator/bootstrap.go b/cmd/machine-config-operator/bootstrap.go index 824301ce64..6d0b407e91 100644 --- a/cmd/machine-config-operator/bootstrap.go +++ b/cmd/machine-config-operator/bootstrap.go @@ -2,14 +2,9 @@ package main import ( "flag" - "fmt" - "io/ioutil" "github.com/golang/glog" "github.com/spf13/cobra" - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/client-go/kubernetes/scheme" "github.com/openshift/machine-config-operator/pkg/operator" "github.com/openshift/machine-config-operator/pkg/version" @@ -103,25 +98,9 @@ func runBootstrapCmd(cmd *cobra.Command, args []string) { bootstrapOpts.infraConfigFile, bootstrapOpts.networkConfigFile, bootstrapOpts.cloudConfigFile, bootstrapOpts.etcdCAFile, bootstrapOpts.etcdMetricCAFile, bootstrapOpts.rootCAFile, bootstrapOpts.kubeCAFile, bootstrapOpts.pullSecretFile, - imgs, + &imgs, bootstrapOpts.destinationDir, ); err != nil { glog.Fatalf("error rendering bootstrap manifests: %v", err) } } - -func rawImagesFromConfigMapOnDisk(file string) ([]byte, error) { - data, err := ioutil.ReadFile(bootstrapOpts.imagesConfigMapFile) - if err != nil { - return nil, err - } - obji, err := runtime.Decode(scheme.Codecs.UniversalDecoder(corev1.SchemeGroupVersion), data) - if err != nil { - return nil, err - } - cm, ok := obji.(*corev1.ConfigMap) - if !ok { - return nil, fmt.Errorf("expected *corev1.ConfigMap found %T", obji) - } - return []byte(cm.Data["images.json"]), nil -} diff --git a/cmd/machine-config-operator/main.go b/cmd/machine-config-operator/main.go index fbccbdf0c3..a4b985dc46 100644 --- a/cmd/machine-config-operator/main.go +++ b/cmd/machine-config-operator/main.go @@ -18,9 +18,6 @@ var ( Short: "Run Machine Config Operator", Long: "", } - - rootOpts struct { - } ) func init() { diff --git a/internal/clients/builder.go b/internal/clients/builder.go index b202492cc9..6f16d67971 100644 --- a/internal/clients/builder.go +++ b/internal/clients/builder.go @@ -13,7 +13,7 @@ import ( ) // Builder can create a variety of kubernetes client interface -// with its embeded rest.Config. +// with its embedded rest.Config. type Builder struct { config *rest.Config } diff --git a/pkg/controller/bootstrap/bootstrap.go b/pkg/controller/bootstrap/bootstrap.go index f98cb0b536..ba8139c6b4 100644 --- a/pkg/controller/bootstrap/bootstrap.go +++ b/pkg/controller/bootstrap/bootstrap.go @@ -16,7 +16,7 @@ import ( yamlutil "k8s.io/apimachinery/pkg/util/yaml" kscheme "k8s.io/client-go/kubernetes/scheme" - "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" + v1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" "github.com/openshift/machine-config-operator/pkg/controller/render" "github.com/openshift/machine-config-operator/pkg/controller/template" "github.com/openshift/machine-config-operator/pkg/generated/clientset/versioned/scheme" diff --git a/pkg/controller/common/helpers.go b/pkg/controller/common/helpers.go index e092087eca..8c246cf8fd 100644 --- a/pkg/controller/common/helpers.go +++ b/pkg/controller/common/helpers.go @@ -1,9 +1,10 @@ package common import ( + "io/ioutil" + igntypes "github.com/coreos/ignition/config/v2_2/types" "github.com/golang/glog" - "io/ioutil" ) // NewIgnConfig returns an empty ignition config with version set as latest version diff --git a/pkg/controller/container-runtime-config/container_runtime_config_controller.go b/pkg/controller/container-runtime-config/container_runtime_config_controller.go index c6cb313be3..5384c12570 100644 --- a/pkg/controller/container-runtime-config/container_runtime_config_controller.go +++ b/pkg/controller/container-runtime-config/container_runtime_config_controller.go @@ -194,7 +194,7 @@ func (ctrl *Controller) imageConfAdded(obj interface{}) { ctrl.imgQueue.Add("openshift-config") } -func (ctrl *Controller) imageConfUpdated(oldObj interface{}, newObj interface{}) { +func (ctrl *Controller) imageConfUpdated(oldObj, newObj interface{}) { ctrl.imgQueue.Add("openshift-config") } @@ -202,7 +202,7 @@ func (ctrl *Controller) imageConfDeleted(obj interface{}) { ctrl.imgQueue.Add("openshift-config") } -func (ctrl *Controller) updateContainerRuntimeConfig(oldObj interface{}, newObj interface{}) { +func (ctrl *Controller) updateContainerRuntimeConfig(oldObj, newObj interface{}) { oldCtrCfg := oldObj.(*mcfgv1.ContainerRuntimeConfig) newCtrCfg := newObj.(*mcfgv1.ContainerRuntimeConfig) @@ -406,7 +406,7 @@ func (ctrl *Controller) syncStatusOnly(cfg *mcfgv1.ContainerRuntimeConfig, err e _, updateErr := ctrl.client.MachineconfigurationV1().ContainerRuntimeConfigs().UpdateStatus(cfg) return updateErr }) - // If an error occured in updating the status just log it + // If an error occurred in updating the status just log it if statusUpdateErr != nil { glog.Warningf("error updating container runtime config status: %v", statusUpdateErr) } @@ -474,7 +474,7 @@ func (ctrl *Controller) syncContainerRuntimeConfig(key string) error { for _, pool := range mcpPools { role := pool.Name // Get MachineConfig - managedKey := getManagedKeyCtrCfg(pool, cfg) + managedKey := getManagedKeyCtrCfg(pool) if err := retry.RetryOnConflict(updateBackoff, func() error { mc, err := ctrl.client.MachineconfigurationV1().MachineConfigs().Get(managedKey, metav1.GetOptions{}) if err != nil && !errors.IsNotFound(err) { @@ -490,13 +490,13 @@ func (ctrl *Controller) syncContainerRuntimeConfig(key string) error { var storageTOML, crioTOML []byte ctrcfg := cfg.Spec.ContainerRuntimeConfig if ctrcfg.OverlaySize != (resource.Quantity{}) { - storageTOML, err = ctrl.mergeConfigChanges(originalStorageIgn, cfg, mc, role, managedKey, isNotFound, updateStorageConfig) + storageTOML, err = ctrl.mergeConfigChanges(originalStorageIgn, cfg, updateStorageConfig) if err != nil { glog.V(2).Infoln(cfg, err, "error merging user changes to storage.conf: %v", err) } } if ctrcfg.LogLevel != "" || ctrcfg.PidsLimit != 0 || ctrcfg.LogSizeMax != (resource.Quantity{}) { - crioTOML, err = ctrl.mergeConfigChanges(originalCRIOIgn, cfg, mc, role, managedKey, isNotFound, updateCRIOConfig) + crioTOML, err = ctrl.mergeConfigChanges(originalCRIOIgn, cfg, updateCRIOConfig) if err != nil { glog.V(2).Infoln(cfg, err, "error merging user changes to crio.conf: %v", err) } @@ -506,6 +506,7 @@ func (ctrl *Controller) syncContainerRuntimeConfig(key string) error { mc = mtmpl.MachineConfigFromIgnConfig(role, managedKey, &tempIgnCfg) } mc.Spec.Config = createNewCtrRuntimeConfigIgnition(storageTOML, crioTOML) + mc.SetAnnotations(map[string]string{ ctrlcommon.GeneratedByControllerVersionAnnotationKey: version.Hash, }) @@ -535,7 +536,7 @@ func (ctrl *Controller) syncContainerRuntimeConfig(key string) error { // mergeConfigChanges retrieves the original/default config data from the templates, decodes it and merges in the changes given by the Custom Resource. // It then encodes the new data and returns it. -func (ctrl *Controller) mergeConfigChanges(origFile *igntypes.File, cfg *mcfgv1.ContainerRuntimeConfig, mc *mcfgv1.MachineConfig, role, managedKey string, isNotFound bool, update updateConfig) ([]byte, error) { +func (ctrl *Controller) mergeConfigChanges(origFile *igntypes.File, cfg *mcfgv1.ContainerRuntimeConfig, update updateConfigFunc) ([]byte, error) { dataURL, err := dataurl.DecodeString(origFile.Contents.Source) if err != nil { return nil, ctrl.syncStatusOnly(cfg, err, "could not decode original Container Runtime config: %v", err) @@ -595,7 +596,7 @@ func (ctrl *Controller) syncImageConfig(key string) error { applied := true role := pool.Name // Get MachineConfig - managedKey := getManagedKeyReg(pool, imgcfg) + managedKey := getManagedKeyReg(pool) if err := retry.RetryOnConflict(updateBackoff, func() error { // Generate the original registries config _, _, originalRegistriesIgn, err := ctrl.generateOriginalContainerRuntimeConfigs(role) @@ -638,7 +639,7 @@ func (ctrl *Controller) syncImageConfig(key string) error { ctrlcommon.GeneratedByControllerVersionAnnotationKey: version.Hash, } mc.ObjectMeta.OwnerReferences = []metav1.OwnerReference{ - metav1.OwnerReference{ + { APIVersion: apicfgv1.SchemeGroupVersion.String(), Kind: "Image", Name: imgcfg.Name, diff --git a/pkg/controller/container-runtime-config/container_runtime_config_controller_test.go b/pkg/controller/container-runtime-config/container_runtime_config_controller_test.go index 80bfeb0180..12ad5a1ede 100644 --- a/pkg/controller/container-runtime-config/container_runtime_config_controller_test.go +++ b/pkg/controller/container-runtime-config/container_runtime_config_controller_test.go @@ -316,7 +316,7 @@ func TestContainerRuntimeConfigCreate(t *testing.T) { mcp2 := helpers.NewMachineConfigPool("worker", nil, helpers.WorkerSelector, "v0") mcp2.ObjectMeta.Labels["custom-crio"] = "storage-config" ctrcfg1 := newContainerRuntimeConfig("set-log-level", &mcfgv1.ContainerRuntimeConfiguration{LogLevel: "debug", LogSizeMax: resource.MustParse("9k"), OverlaySize: resource.MustParse("3G")}, metav1.AddLabelToSelector(&metav1.LabelSelector{}, "custom-crio", "my-config")) - mcs1 := helpers.NewMachineConfig(getManagedKeyCtrCfg(mcp, ctrcfg1), map[string]string{"node-role/master": ""}, "dummy://", []igntypes.File{{}}) + mcs1 := helpers.NewMachineConfig(getManagedKeyCtrCfg(mcp), map[string]string{"node-role": "master"}, "dummy://", []igntypes.File{{}}) f.ccLister = append(f.ccLister, cc) f.mcpLister = append(f.mcpLister, mcp) @@ -349,7 +349,7 @@ func TestContainerRuntimeConfigUpdate(t *testing.T) { mcp2 := helpers.NewMachineConfigPool("worker", nil, helpers.WorkerSelector, "v0") mcp2.ObjectMeta.Labels["custom-crio"] = "storage-config" ctrcfg1 := newContainerRuntimeConfig("set-log-level", &mcfgv1.ContainerRuntimeConfiguration{LogLevel: "debug", LogSizeMax: resource.MustParse("9k"), OverlaySize: resource.MustParse("3G")}, metav1.AddLabelToSelector(&metav1.LabelSelector{}, "custom-crio", "my-config")) - mcs := helpers.NewMachineConfig(getManagedKeyCtrCfg(mcp, ctrcfg1), map[string]string{"node-role/master": ""}, "dummy://", []igntypes.File{{}}) + mcs := helpers.NewMachineConfig(getManagedKeyCtrCfg(mcp), map[string]string{"node-role": "master"}, "dummy://", []igntypes.File{{}}) f.ccLister = append(f.ccLister, cc) f.mcpLister = append(f.mcpLister, mcp) @@ -425,8 +425,8 @@ func TestImageConfigCreate(t *testing.T) { mcp2 := helpers.NewMachineConfigPool("worker", nil, helpers.WorkerSelector, "v0") imgcfg1 := newImageConfig("cluster", &apicfgv1.RegistrySources{InsecureRegistries: []string{"blah.io"}}) cvcfg1 := newClusterVersionConfig("version", "test.io/myuser/myimage:test") - mcs1 := helpers.NewMachineConfig(getManagedKeyReg(mcp, imgcfg1), map[string]string{"node-role/master": ""}, "dummy://", []igntypes.File{{}}) - mcs2 := helpers.NewMachineConfig(getManagedKeyReg(mcp2, imgcfg1), map[string]string{"node-role/worker": ""}, "dummy://", []igntypes.File{{}}) + mcs1 := helpers.NewMachineConfig(getManagedKeyReg(mcp), map[string]string{"node-role": "master"}, "dummy://", []igntypes.File{{}}) + mcs2 := helpers.NewMachineConfig(getManagedKeyReg(mcp2), map[string]string{"node-role": "worker"}, "dummy://", []igntypes.File{{}}) f.ccLister = append(f.ccLister, cc) f.mcpLister = append(f.mcpLister, mcp) @@ -457,8 +457,8 @@ func TestImageConfigUpdate(t *testing.T) { mcp2 := helpers.NewMachineConfigPool("worker", nil, helpers.WorkerSelector, "v0") imgcfg1 := newImageConfig("cluster", &apicfgv1.RegistrySources{InsecureRegistries: []string{"blah.io"}}) cvcfg1 := newClusterVersionConfig("version", "test.io/myuser/myimage:test") - mcs1 := helpers.NewMachineConfig(getManagedKeyReg(mcp, imgcfg1), map[string]string{"node-role/master": ""}, "dummy://", []igntypes.File{{}}) - mcs2 := helpers.NewMachineConfig(getManagedKeyReg(mcp2, imgcfg1), map[string]string{"node-role/worker": ""}, "dummy://", []igntypes.File{{}}) + mcs1 := helpers.NewMachineConfig(getManagedKeyReg(mcp), map[string]string{"node-role": "master"}, "dummy://", []igntypes.File{{}}) + mcs2 := helpers.NewMachineConfig(getManagedKeyReg(mcp2), map[string]string{"node-role": "worker"}, "dummy://", []igntypes.File{{}}) f.ccLister = append(f.ccLister, cc) f.mcpLister = append(f.mcpLister, mcp) diff --git a/pkg/controller/container-runtime-config/helpers.go b/pkg/controller/container-runtime-config/helpers.go index 50253fd01e..50be7765c0 100644 --- a/pkg/controller/container-runtime-config/helpers.go +++ b/pkg/controller/container-runtime-config/helpers.go @@ -28,7 +28,7 @@ const ( registriesConfigPath = "/etc/containers/registries.conf" ) -var errParsingReference error = errors.New("error parsing reference of desired image from cluster version config") +var errParsingReference = errors.New("error parsing reference of desired image from cluster version config") // TOML-friendly explicit tables used for conversions. type tomlConfigStorage struct { @@ -55,11 +55,11 @@ type tomlConfigCRIO struct { type tomlConfigRegistries struct { Registries []sysregistriesv2.Registry `toml:"registry"` - // backwards compatability to sysregistries v1 + // backwards compatibility to sysregistries v1 sysregistriesv2.V1TOMLConfig `toml:"registries"` } -type updateConfig func(data []byte, internal *mcfgv1.ContainerRuntimeConfiguration) ([]byte, error) +type updateConfigFunc func(data []byte, internal *mcfgv1.ContainerRuntimeConfiguration) ([]byte, error) func createNewCtrRuntimeConfigIgnition(storageTOMLConfig, crioTOMLConfig []byte) igntypes.Config { tempIgnConfig := ctrlcommon.NewIgnConfig() @@ -132,6 +132,7 @@ func createNewRegistriesConfigIgnition(registriesTOMLConfig []byte) igntypes.Con func findStorageConfig(mc *mcfgv1.MachineConfig) (*igntypes.File, error) { for _, c := range mc.Spec.Config.Storage.Files { if c.Path == storageConfigPath { + c := c return &c, nil } } @@ -141,6 +142,7 @@ func findStorageConfig(mc *mcfgv1.MachineConfig) (*igntypes.File, error) { func findCRIOConfig(mc *mcfgv1.MachineConfig) (*igntypes.File, error) { for _, c := range mc.Spec.Config.Storage.Files { if c.Path == crioConfigPath { + c := c return &c, nil } } @@ -156,11 +158,11 @@ func findRegistriesConfig(mc *mcfgv1.MachineConfig) (*igntypes.File, error) { return nil, fmt.Errorf("could not find Registries Config") } -func getManagedKeyCtrCfg(pool *mcfgv1.MachineConfigPool, config *mcfgv1.ContainerRuntimeConfig) string { +func getManagedKeyCtrCfg(pool *mcfgv1.MachineConfigPool) string { return fmt.Sprintf("99-%s-%s-containerruntime", pool.Name, pool.ObjectMeta.UID) } -func getManagedKeyReg(pool *mcfgv1.MachineConfigPool, config *apicfgv1.Image) string { +func getManagedKeyReg(pool *mcfgv1.MachineConfigPool) string { return fmt.Sprintf("99-%s-%s-registries", pool.Name, pool.ObjectMeta.UID) } diff --git a/pkg/controller/kubelet-config/kubelet_config_controller.go b/pkg/controller/kubelet-config/kubelet_config_controller.go index 3a3e373433..dda00d3136 100644 --- a/pkg/controller/kubelet-config/kubelet_config_controller.go +++ b/pkg/controller/kubelet-config/kubelet_config_controller.go @@ -470,6 +470,7 @@ func (ctrl *Controller) syncKubeletConfig(key string) error { mc = mtmpl.MachineConfigFromIgnConfig(role, managedKey, &ignConfig) } mc.Spec.Config = createNewKubeletIgnition(cfgYAML) + mc.SetAnnotations(map[string]string{ ctrlcommon.GeneratedByControllerVersionAnnotationKey: version.Hash, }) diff --git a/pkg/controller/kubelet-config/kubelet_config_features.go b/pkg/controller/kubelet-config/kubelet_config_features.go index 6b9a68b8ec..863f7f0fbf 100644 --- a/pkg/controller/kubelet-config/kubelet_config_features.go +++ b/pkg/controller/kubelet-config/kubelet_config_features.go @@ -69,6 +69,9 @@ func (ctrl *Controller) syncFeatureHandler(key string) error { return err } featureGates, err := ctrl.generateFeatureMap(features) + if err != nil { + return err + } // Find all MachineConfigPools mcpPools, err := ctrl.mcpLister.List(labels.Everything()) @@ -180,6 +183,7 @@ func (ctrl *Controller) deleteFeature(obj interface{}) { glog.V(4).Infof("Deleted Feature %s and restored default config", features.Name) } +//nolint:gocritic func (ctrl *Controller) generateFeatureMap(features *osev1.FeatureGate) (*map[string]bool, error) { rv := make(map[string]bool) set, ok := osev1.FeatureSets[features.Spec.FeatureSet] diff --git a/pkg/controller/node/node_controller.go b/pkg/controller/node/node_controller.go index e18ed1bde8..fb2cd3a4c0 100644 --- a/pkg/controller/node/node_controller.go +++ b/pkg/controller/node/node_controller.go @@ -45,9 +45,6 @@ const ( updateDelay = 5 * time.Second ) -// controllerKind contains the schema.GroupVersionKind for this controller type. -var controllerKind = mcfgv1.SchemeGroupVersion.WithKind("MachineConfigPool") - var nodeUpdateBackoff = wait.Backoff{ Steps: 5, Duration: 100 * time.Millisecond, diff --git a/pkg/controller/node/status.go b/pkg/controller/node/status.go index 0b666a8f36..4f989be5a8 100644 --- a/pkg/controller/node/status.go +++ b/pkg/controller/node/status.go @@ -126,7 +126,7 @@ func isNodeManaged(node *corev1.Node) bool { return true } -/// isNodeDone returns true if the current == desired and the MCD has marked done. +// isNodeDone returns true if the current == desired and the MCD has marked done. func isNodeDone(node *corev1.Node) bool { if node.Annotations == nil { return false diff --git a/pkg/controller/render/hash.go b/pkg/controller/render/hash.go index 7fb664125b..36d857a0e8 100644 --- a/pkg/controller/render/hash.go +++ b/pkg/controller/render/hash.go @@ -1,6 +1,7 @@ package render import ( + //nolint:gosec "crypto/md5" "fmt" @@ -40,6 +41,7 @@ func getMachineConfigHashedName(pool *mcfgv1.MachineConfigPool, config *mcfgv1.M } func hashData(data []byte) ([]byte, error) { + //nolint:gosec hasher := md5.New() if _, err := hasher.Write(salt); err != nil { return nil, fmt.Errorf("error computing hash: %v", err) diff --git a/pkg/controller/template/render.go b/pkg/controller/template/render.go index a761698069..0bda6f2245 100644 --- a/pkg/controller/template/render.go +++ b/pkg/controller/template/render.go @@ -95,7 +95,7 @@ func generateTemplateMachineConfigs(config *RenderConfig, templateDir string) ([ } // GenerateMachineConfigsForRole creates MachineConfigs for the role provided -func GenerateMachineConfigsForRole(config *RenderConfig, role string, templateDir string) ([]*mcfgv1.MachineConfig, error) { +func GenerateMachineConfigsForRole(config *RenderConfig, role, templateDir string) ([]*mcfgv1.MachineConfig, error) { path := filepath.Join(templateDir, role) infos, err := ioutil.ReadDir(path) if err != nil { @@ -259,7 +259,7 @@ const ( ) // MachineConfigFromIgnConfig creates a MachineConfig with the provided Ignition config -func MachineConfigFromIgnConfig(role string, name string, ignCfg *igntypes.Config) *mcfgv1.MachineConfig { +func MachineConfigFromIgnConfig(role, name string, ignCfg *igntypes.Config) *mcfgv1.MachineConfig { labels := map[string]string{ machineConfigRoleLabelKey: role, } @@ -331,7 +331,7 @@ func renderTemplate(config RenderConfig, path string, b []byte) ([]byte, error) var skipKeyValidate = regexp.MustCompile(`^[_a-z]\w*$`) -// Keys labled with skip ie. {{skip "key"}}, don't need to be templated in now because at Ignition request they will be templated in with query params +// Keys labelled with skip ie. {{skip "key"}}, don't need to be templated in now because at Ignition request they will be templated in with query params func skipMissing(key string) (interface{}, error) { if !skipKeyValidate.Match([]byte(key)) { return nil, fmt.Errorf("invalid key for skipKey") @@ -385,7 +385,7 @@ func cloudProvider(cfg RenderConfig) (interface{}, error) { // // [1]: https://kubernetes.io/docs/reference/command-line-tools-reference/kubelet/#options func cloudConfigFlag(cfg RenderConfig) interface{} { - if len(cfg.CloudProviderConfig) == 0 { + if cfg.CloudProviderConfig == "" { return "" } flag := "--cloud-config=/etc/kubernetes/cloud.conf" diff --git a/pkg/controller/template/status.go b/pkg/controller/template/status.go index bcbf5e3511..1307cde3ef 100644 --- a/pkg/controller/template/status.go +++ b/pkg/controller/template/status.go @@ -35,7 +35,7 @@ func (ctrl *Controller) syncRunningStatus(ctrlconfig *mcfgv1.ControllerConfig) e // - sets the `failing` condition to `true` using the `oerr` func (ctrl *Controller) syncFailingStatus(ctrlconfig *mcfgv1.ControllerConfig, oerr error) error { if oerr == nil { - return oerr + return nil } updateFunc := func(cfg *mcfgv1.ControllerConfig) error { reason := oerr.Error() diff --git a/pkg/controller/template/template_controller.go b/pkg/controller/template/template_controller.go index df4c464d92..41ad75b974 100644 --- a/pkg/controller/template/template_controller.go +++ b/pkg/controller/template/template_controller.go @@ -166,7 +166,7 @@ func (ctrl *Controller) deleteSecret(obj interface{}) { } glog.V(4).Infof("Re-syncing ControllerConfig %s due to secret deletion", cfg.Name) // TODO(runcom): should we resync w/o a secret which is going to just cause the controller to fail when trying to get the secret itself? - //ctrl.enqueueControllerConfig(cfg) + // ctrl.enqueueControllerConfig(cfg) } } diff --git a/pkg/daemon/daemon.go b/pkg/daemon/daemon.go index ffce0ea283..af99a8b8e5 100644 --- a/pkg/daemon/daemon.go +++ b/pkg/daemon/daemon.go @@ -132,9 +132,8 @@ const ( // also retrieve the pending config after a reboot pendingStateMessageID = "machine-config-daemon-pending-state" - kubeletHealthzEndpoint = "http://localhost:10248/healthz" - kubeletHealthzPollingInterval = time.Duration(30 * time.Second) - kubeletHealthzTimeout = time.Duration(30 * time.Second) + kubeletHealthzPollingInterval = 30 * time.Second + kubeletHealthzTimeout = 30 * time.Second kubeletHealthzFailureThreshold = 3 // updateDelay is the baseline speed at which we react to changes. We don't @@ -175,7 +174,7 @@ func rebootCommand(rationale string) *exec.Cmd { // New sets up the systemd and kubernetes connections needed to update the // machine. func New( - nodeName string, + nodeName, operatingSystem string, nodeUpdaterClient NodeUpdaterClient, bootID, @@ -447,13 +446,13 @@ func (dn *Daemon) runOnceFrom() error { glog.Warningf("Unable to decipher onceFrom config type: %s", err) return err } - switch configi.(type) { + switch c := configi.(type) { case igntypes.Config: glog.V(2).Info("Daemon running directly from Ignition") - return dn.runOnceFromIgnition(configi.(igntypes.Config)) + return dn.runOnceFromIgnition(c) case mcfgv1.MachineConfig: glog.V(2).Info("Daemon running directly from MachineConfig") - return dn.runOnceFromMachineConfig(configi.(mcfgv1.MachineConfig), contentFrom) + return dn.runOnceFromMachineConfig(c, contentFrom) } return errors.New("unsupported onceFrom type provided") } @@ -468,6 +467,7 @@ func (dn *Daemon) InstallSignalHandler(signaled chan struct{}) { // https://github.com/openshift/machine-config-operator/issues/407 go func() { for sig := range termChan { + //nolint:gocritic switch sig { case syscall.SIGTERM: dn.updateActiveLock.Lock() @@ -727,7 +727,7 @@ func (dn *Daemon) LogSystemData() { glog.Info(status) } - boots, err := RunGetOut("journalctl", "--list-boots") + boots, err := runGetOut("journalctl", "--list-boots") if err != nil { glog.Errorf("Listing boots: %v", err) } @@ -767,7 +767,7 @@ func (dn *Daemon) getPendingConfig() (string, error) { return "", nil } var p pendingConfigState - if err := json.Unmarshal([]byte(s), &p); err != nil { + if err := json.Unmarshal(s, &p); err != nil { return "", errors.Wrapf(err, "parsing transient state") } @@ -1051,7 +1051,7 @@ func (dn *Daemon) completeUpdate(node *corev1.Node, desiredConfigName string) er // triggerUpdateWithMachineConfig starts the update. It queries the cluster for // the current and desired config if they weren't passed. -func (dn *Daemon) triggerUpdateWithMachineConfig(currentConfig *mcfgv1.MachineConfig, desiredConfig *mcfgv1.MachineConfig) error { +func (dn *Daemon) triggerUpdateWithMachineConfig(currentConfig, desiredConfig *mcfgv1.MachineConfig) error { if currentConfig == nil { ccAnnotation, err := getNodeAnnotation(dn.node, constants.CurrentMachineConfigAnnotationKey) if err != nil { diff --git a/pkg/daemon/fake_writer.go b/pkg/daemon/fake_writer.go index 58d9f92b20..9a8a240dad 100644 --- a/pkg/daemon/fake_writer.go +++ b/pkg/daemon/fake_writer.go @@ -20,7 +20,7 @@ func (nw *fakeNodeWriter) Run(stop <-chan struct{}) { } // SetDone sets the state to Done. -func (nw *fakeNodeWriter) SetDone(client corev1client.NodeInterface, lister corev1lister.NodeLister, node string, dcAnnotation string) error { +func (nw *fakeNodeWriter) SetDone(client corev1client.NodeInterface, lister corev1lister.NodeLister, node, dcAnnotation string) error { return nil } diff --git a/pkg/daemon/rpm-ostree.go b/pkg/daemon/rpm-ostree.go index 2cd883c23e..7b7024282d 100644 --- a/pkg/daemon/rpm-ostree.go +++ b/pkg/daemon/rpm-ostree.go @@ -16,20 +16,15 @@ import ( "github.com/openshift/machine-config-operator/pkg/daemon/constants" ) -const ( - pivotUnit = "pivot.service" - rpmostreedUnit = "rpm-ostreed.service" -) - -// RpmOstreeState houses zero or more RpmOstreeDeployments +// rpmOstreeState houses zero or more RpmOstreeDeployments // Subset of `rpm-ostree status --json` // https://github.com/projectatomic/rpm-ostree/blob/bce966a9812df141d38e3290f845171ec745aa4e/src/daemon/rpmostreed-deployment-utils.c#L227 -type RpmOstreeState struct { - Deployments []RpmOstreeDeployment +type rpmOstreeState struct { + Deployments []rpmOstreeDeployment } -// RpmOstreeDeployment represents a single deployment on a node -type RpmOstreeDeployment struct { +// rpmOstreeDeployment represents a single deployment on a node +type rpmOstreeDeployment struct { ID string `json:"id"` OSName string `json:"osname"` Serial int32 `json:"serial"` @@ -49,6 +44,8 @@ type NodeUpdaterClient interface { RunPivot(string) error } +// TODO(runcom): make this private to pkg/daemon!!! +// // RpmOstreeClient provides all RpmOstree related methods in one structure. // This structure implements DeploymentClient type RpmOstreeClient struct{} @@ -59,9 +56,9 @@ func NewNodeUpdaterClient() NodeUpdaterClient { } // getBootedDeployment returns the current deployment found -func (r *RpmOstreeClient) getBootedDeployment() (*RpmOstreeDeployment, error) { - var rosState RpmOstreeState - output, err := RunGetOut("rpm-ostree", "status", "--json") +func (r *RpmOstreeClient) getBootedDeployment() (*rpmOstreeDeployment, error) { + var rosState rpmOstreeState + output, err := runGetOut("rpm-ostree", "status", "--json") if err != nil { return nil, err } @@ -72,6 +69,7 @@ func (r *RpmOstreeClient) getBootedDeployment() (*RpmOstreeDeployment, error) { for _, deployment := range rosState.Deployments { if deployment.Booted { + deployment := deployment return &deployment, nil } } @@ -81,7 +79,7 @@ func (r *RpmOstreeClient) getBootedDeployment() (*RpmOstreeDeployment, error) { // GetStatus returns multi-line human-readable text describing system status func (r *RpmOstreeClient) GetStatus() (string, error) { - output, err := RunGetOut("rpm-ostree", "status") + output, err := runGetOut("rpm-ostree", "status") if err != nil { return "", err } @@ -146,3 +144,15 @@ func followPivotJournalLogs(stopCh <-chan time.Time) { cmd.Process.Kill() }() } + +// runGetOut executes a command, logging it, and return the stdout output. +func runGetOut(command string, args ...string) ([]byte, error) { + glog.Infof("Running captured: %s %s\n", command, strings.Join(args, " ")) + cmd := exec.Command(command, args...) + cmd.Stderr = os.Stderr + rawOut, err := cmd.Output() + if err != nil { + return nil, err + } + return rawOut, nil +} diff --git a/pkg/daemon/run.go b/pkg/daemon/run.go deleted file mode 100644 index 84b1426da8..0000000000 --- a/pkg/daemon/run.go +++ /dev/null @@ -1,30 +0,0 @@ -package daemon - -import ( - "os" - "os/exec" - "strings" - - "github.com/golang/glog" -) - -// Run executes a command, logging it. -func Run(command string, args ...string) error { - glog.Infof("Running: %s %s\n", command, strings.Join(args, " ")) - cmd := exec.Command(command, args...) - cmd.Stdout = os.Stdout - cmd.Stderr = os.Stderr - return cmd.Run() -} - -// RunGetOut executes a command, logging it, and return the stdout output. -func RunGetOut(command string, args ...string) ([]byte, error) { - glog.Infof("Running captured: %s %s\n", command, strings.Join(args, " ")) - cmd := exec.Command(command, args...) - cmd.Stderr = os.Stderr - rawOut, err := cmd.Output() - if err != nil { - return nil, err - } - return rawOut, nil -} diff --git a/pkg/daemon/update.go b/pkg/daemon/update.go index fa956adf0d..fa8c8a4abe 100644 --- a/pkg/daemon/update.go +++ b/pkg/daemon/update.go @@ -25,7 +25,6 @@ import ( errors "github.com/pkg/errors" "github.com/vincent-petithory/dataurl" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/wait" ) @@ -78,7 +77,7 @@ func getNodeRef(node *corev1.Node) *corev1.ObjectReference { return &corev1.ObjectReference{ Kind: "Node", Name: node.GetName(), - UID: types.UID(node.GetUID()), + UID: node.GetUID(), } } @@ -312,7 +311,7 @@ func Reconcilable(oldConfig, newConfig *mcfgv1.MachineConfig) (*MachineConfigDif if !reflect.DeepEqual(oldIgn.Passwd.Users, newIgn.Passwd.Users) { // check if the prior config is empty and that this is the first time running. // if so, the SSHKey from the cluster config and user "core" must be added to machine config. - if len(oldIgn.Passwd.Users) >= 0 && len(newIgn.Passwd.Users) >= 1 { + if len(oldIgn.Passwd.Users) > 0 && len(newIgn.Passwd.Users) >= 1 { // there is an update to Users, we must verify that it is ONLY making an acceptable // change to the SSHAuthorizedKeys for the user "core" for _, user := range newIgn.Passwd.Users { diff --git a/pkg/daemon/writer.go b/pkg/daemon/writer.go index 6d5c7d8189..2acf19452b 100644 --- a/pkg/daemon/writer.go +++ b/pkg/daemon/writer.go @@ -70,7 +70,7 @@ func (nw *clusterNodeWriter) Run(stop <-chan struct{}) { } // SetDone sets the state to Done. -func (nw *clusterNodeWriter) SetDone(client corev1client.NodeInterface, lister corev1lister.NodeLister, node string, dcAnnotation string) error { +func (nw *clusterNodeWriter) SetDone(client corev1client.NodeInterface, lister corev1lister.NodeLister, node, dcAnnotation string) error { annos := map[string]string{ constants.MachineConfigDaemonStateAnnotationKey: constants.MachineConfigDaemonStateDone, constants.CurrentMachineConfigAnnotationKey: dcAnnotation, diff --git a/pkg/operator/bootstrap.go b/pkg/operator/bootstrap.go index 22636b8506..b4dab68f9c 100644 --- a/pkg/operator/bootstrap.go +++ b/pkg/operator/bootstrap.go @@ -19,11 +19,11 @@ import ( // RenderBootstrap writes to destinationDir static Pods. func RenderBootstrap( - clusterConfigConfigMapFile string, - infraFile, networkFile string, - cloudConfigFile string, - etcdCAFile, etcdMetricCAFile string, rootCAFile string, kubeAPIServerServingCA string, pullSecretFile string, - imgs Images, + clusterConfigConfigMapFile, + infraFile, networkFile, + cloudConfigFile, + etcdCAFile, etcdMetricCAFile, rootCAFile, kubeAPIServerServingCA, pullSecretFile string, + imgs *Images, destinationDir string, ) error { filesData := map[string][]byte{} @@ -137,15 +137,16 @@ func RenderBootstrap( for _, m := range manifests { var b []byte var err error - if len(m.name) > 0 { + switch { + case len(m.name) > 0: glog.Info(m.name) b, err = renderAsset(config, m.name) if err != nil { return err } - } else if len(m.data) > 0 { + case len(m.data) > 0: b = m.data - } else { + default: continue } diff --git a/pkg/operator/operator.go b/pkg/operator/operator.go index 7dbb9696b8..cb251ceed8 100644 --- a/pkg/operator/operator.go +++ b/pkg/operator/operator.go @@ -108,8 +108,7 @@ type Operator struct { // New returns a new machine config operator. func New( - namespace, name string, - imagesFile string, + namespace, name, imagesFile string, mcpInformer mcfginformersv1.MachineConfigPoolInformer, mcInformer mcfginformersv1.MachineConfigInformer, controllerConfigInformer mcfginformersv1.ControllerConfigInformer, @@ -119,7 +118,7 @@ func New( daemonsetInformer appsinformersv1.DaemonSetInformer, clusterRoleInformer rbacinformersv1.ClusterRoleInformer, clusterRoleBindingInformer rbacinformersv1.ClusterRoleBindingInformer, - mcoCmInformer coreinformersv1.ConfigMapInformer, + mcoCmInformer, clusterCmInfomer coreinformersv1.ConfigMapInformer, infraInformer configinformersv1.InfrastructureInformer, networkInformer configinformersv1.NetworkInformer, @@ -406,7 +405,7 @@ func (optr *Operator) sync(key string) error { } // create renderConfig - rc := getRenderConfig(namespace, string(kubeAPIServerServingCABytes), spec, imgs, infra.Status.APIServerInternalURL) + rc := getRenderConfig(namespace, string(kubeAPIServerServingCABytes), spec, &imgs, infra.Status.APIServerURL) // syncFuncs is the list of sync functions that are executed in order. // any error marks sync as failure but continues to next syncFunc var syncFuncs = []syncFunc{ @@ -454,9 +453,8 @@ func (optr *Operator) getCloudConfigFromConfigMap(namespace, name, key string) ( } if cc, ok := cm.Data[key]; ok { return cc, nil - } else { - return "", fmt.Errorf("%s not found in %s/%s", key, namespace, name) } + return "", fmt.Errorf("%s not found in %s/%s", key, namespace, name) } // getGlobalConfig gets global configuration for the cluster, namely, the Infrastructure and Network types. @@ -473,8 +471,8 @@ func (optr *Operator) getGlobalConfig() (*configv1.Infrastructure, *configv1.Net return infra, network, nil } -func getRenderConfig(tnamespace, kubeAPIServerServingCA string, ccSpec *mcfgv1.ControllerConfigSpec, imgs Images, apiServerURL string) renderConfig { - return renderConfig{ +func getRenderConfig(tnamespace, kubeAPIServerServingCA string, ccSpec *mcfgv1.ControllerConfigSpec, imgs *Images, apiServerURL string) *renderConfig { + return &renderConfig{ TargetNamespace: tnamespace, Version: version.Raw, ControllerConfig: *ccSpec, diff --git a/pkg/operator/render.go b/pkg/operator/render.go index 39afa96059..91c4d22fff 100644 --- a/pkg/operator/render.go +++ b/pkg/operator/render.go @@ -21,11 +21,11 @@ type renderConfig struct { Version string ControllerConfig mcfgv1.ControllerConfigSpec APIServerURL string - Images Images + Images *Images KubeAPIServerServingCA string } -func renderAsset(config renderConfig, path string) ([]byte, error) { +func renderAsset(config *renderConfig, path string) ([]byte, error) { objBytes, err := assets.Asset(path) if err != nil { return nil, fmt.Errorf("error getting asset %s: %v", path, err) diff --git a/pkg/operator/status.go b/pkg/operator/status.go index ec89f98e97..55f783bf5a 100644 --- a/pkg/operator/status.go +++ b/pkg/operator/status.go @@ -12,7 +12,6 @@ import ( "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" - "k8s.io/apimachinery/pkg/types" mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" @@ -39,7 +38,7 @@ func (optr *Operator) syncVersion() error { Kind: co.Kind, Name: co.Name, Namespace: co.Namespace, - UID: types.UID(co.GetUID()), + UID: co.GetUID(), } optr.eventRecorder.Eventf(mcoObjectRef, corev1.EventTypeNormal, "OperatorVersionChanged", fmt.Sprintf("clusteroperator/machine-config-operator version changed from %v to %v", co.Status.Versions, optr.vStore.GetAll())) } @@ -102,7 +101,7 @@ func (optr *Operator) syncProgressingStatus() error { Kind: co.Kind, Name: co.Name, Namespace: co.Namespace, - UID: types.UID(co.GetUID()), + UID: co.GetUID(), } ) if optr.vStore.Equal(co.Status.Versions) { @@ -220,14 +219,14 @@ func (optr *Operator) initializeClusterOperator() (*configv1.ClusterOperator, er // setOperatorStatusExtension sets the raw extension field of the clusteroperator. Today, we set // the MCPs statuses and an optional status error which we may get during a sync. -func (optr *Operator) setOperatorStatusExtension(status *configv1.ClusterOperatorStatus, err error) { +func (optr *Operator) setOperatorStatusExtension(status *configv1.ClusterOperatorStatus, statusErr error) { statuses, err := optr.allMachineConfigPoolStatus() if err != nil { glog.Error(err) return } - if err != nil { - statuses["lastSyncError"] = err.Error() + if statusErr != nil { + statuses["lastSyncError"] = statusErr.Error() } raw, err := json.Marshal(statuses) if err != nil { @@ -258,10 +257,10 @@ func (optr *Operator) allMachineConfigPoolStatus() (map[string]string, error) { // isMachineConfigPoolConfigurationValid returns nil error when the configuration of a `pool` is created by the controller at version `version`. func isMachineConfigPoolConfigurationValid(pool *mcfgv1.MachineConfigPool, version string, machineConfigGetter func(string) (*mcfgv1.MachineConfig, error)) error { // both .status.configuration.name and .status.configuration.source must be set. - if len(pool.Spec.Configuration.Name) == 0 { + if pool.Spec.Configuration.Name == "" { return fmt.Errorf("configuration spec for pool %s is empty", pool.GetName()) } - if len(pool.Status.Configuration.Name) == 0 { + if pool.Status.Configuration.Name == "" { return fmt.Errorf("configuration status for pool %s is empty", pool.GetName()) } if len(pool.Status.Configuration.Source) == 0 { diff --git a/pkg/operator/status_test.go b/pkg/operator/status_test.go index 9fca7130b6..9ad26574d7 100644 --- a/pkg/operator/status_test.go +++ b/pkg/operator/status_test.go @@ -171,7 +171,7 @@ func TestOperatorSyncStatus(t *testing.T) { syncFuncs: []syncFunc{ { name: "fn1", - fn: func(config renderConfig) error { return errors.New("got err") }, + fn: func(config *renderConfig) error { return errors.New("got err") }, }, }, expectOperatorFail: true, @@ -195,7 +195,7 @@ func TestOperatorSyncStatus(t *testing.T) { syncFuncs: []syncFunc{ { name: "fn1", - fn: func(config renderConfig) error { return nil }, + fn: func(config *renderConfig) error { return nil }, }, }, }, @@ -223,7 +223,7 @@ func TestOperatorSyncStatus(t *testing.T) { syncFuncs: []syncFunc{ { name: "fn1", - fn: func(config renderConfig) error { return nil }, + fn: func(config *renderConfig) error { return nil }, }, }, }, @@ -245,7 +245,7 @@ func TestOperatorSyncStatus(t *testing.T) { syncFuncs: []syncFunc{ { name: "fn1", - fn: func(config renderConfig) error { return nil }, + fn: func(config *renderConfig) error { return nil }, }, }, }, @@ -273,7 +273,7 @@ func TestOperatorSyncStatus(t *testing.T) { syncFuncs: []syncFunc{ { name: "fn1", - fn: func(config renderConfig) error { return nil }, + fn: func(config *renderConfig) error { return nil }, }, }, }, @@ -301,7 +301,7 @@ func TestOperatorSyncStatus(t *testing.T) { syncFuncs: []syncFunc{ { name: "fn1", - fn: func(config renderConfig) error { return nil }, + fn: func(config *renderConfig) error { return nil }, }, }, }, @@ -325,7 +325,7 @@ func TestOperatorSyncStatus(t *testing.T) { syncFuncs: []syncFunc{ { name: "fn1", - fn: func(config renderConfig) error { return errors.New("mock error") }, + fn: func(config *renderConfig) error { return errors.New("mock error") }, }, }, }, @@ -354,7 +354,7 @@ func TestOperatorSyncStatus(t *testing.T) { syncFuncs: []syncFunc{ { name: "fn1", - fn: func(config renderConfig) error { return errors.New("error") }, + fn: func(config *renderConfig) error { return errors.New("error") }, }, }, }, @@ -379,7 +379,7 @@ func TestOperatorSyncStatus(t *testing.T) { syncFuncs: []syncFunc{ { name: "fn1", - fn: func(config renderConfig) error { return errors.New("error") }, + fn: func(config *renderConfig) error { return errors.New("error") }, }, }, }, @@ -407,7 +407,7 @@ func TestOperatorSyncStatus(t *testing.T) { syncFuncs: []syncFunc{ { name: "fn1", - fn: func(config renderConfig) error { return nil }, + fn: func(config *renderConfig) error { return nil }, }, }, }, @@ -430,7 +430,7 @@ func TestOperatorSyncStatus(t *testing.T) { syncFuncs: []syncFunc{ { name: "fn1", - fn: func(config renderConfig) error { return errors.New("error") }, + fn: func(config *renderConfig) error { return errors.New("error") }, }, }, }, @@ -452,7 +452,7 @@ func TestOperatorSyncStatus(t *testing.T) { syncFuncs: []syncFunc{ { name: "fn1", - fn: func(config renderConfig) error { return nil }, + fn: func(config *renderConfig) error { return nil }, }, }, }, @@ -479,7 +479,7 @@ func TestOperatorSyncStatus(t *testing.T) { optr.vStore.Set("operator", "test-version") } optr.configClient = fakeconfigclientset.NewSimpleClientset(co) - err := optr.syncAll(renderConfig{}, sync.syncFuncs) + err := optr.syncAll(&renderConfig{}, sync.syncFuncs) if sync.expectOperatorFail { assert.NotNil(t, err, "test case %d, sync call %d, expected an error", idx, j) } else { @@ -515,18 +515,18 @@ func TestInClusterBringUpStayOnErr(t *testing.T) { optr.configClient = fakeconfigclientset.NewSimpleClientset(co) optr.inClusterBringup = true - fn1 := func(config renderConfig) error { + fn1 := func(config *renderConfig) error { return errors.New("mocked fn1") } - err := optr.syncAll(renderConfig{}, []syncFunc{{name: "mock1", fn: fn1}}) + err := optr.syncAll(&renderConfig{}, []syncFunc{{name: "mock1", fn: fn1}}) assert.NotNil(t, err, "expected syncAll to fail") assert.True(t, optr.inClusterBringup) - fn1 = func(config renderConfig) error { + fn1 = func(config *renderConfig) error { return nil } - err = optr.syncAll(renderConfig{}, []syncFunc{{name: "mock1", fn: fn1}}) + err = optr.syncAll(&renderConfig{}, []syncFunc{{name: "mock1", fn: fn1}}) assert.Nil(t, err, "expected syncAll to pass") assert.False(t, optr.inClusterBringup) diff --git a/pkg/operator/sync.go b/pkg/operator/sync.go index 9b0e287efe..274aa44cf9 100644 --- a/pkg/operator/sync.go +++ b/pkg/operator/sync.go @@ -24,10 +24,10 @@ import ( type syncFunc struct { name string - fn func(config renderConfig) error + fn func(config *renderConfig) error } -func (optr *Operator) syncAll(rconfig renderConfig, syncFuncs []syncFunc) error { +func (optr *Operator) syncAll(rconfig *renderConfig, syncFuncs []syncFunc) error { if err := optr.syncProgressingStatus(); err != nil { return fmt.Errorf("error syncing progressing status: %v", err) } @@ -91,7 +91,7 @@ func (optr *Operator) syncCustomResourceDefinitions() error { return nil } -func (optr *Operator) syncMachineConfigPools(config renderConfig) error { +func (optr *Operator) syncMachineConfigPools(config *renderConfig) error { mcps := []string{ "manifests/master.machineconfigpool.yaml", "manifests/worker.machineconfigpool.yaml", @@ -112,7 +112,7 @@ func (optr *Operator) syncMachineConfigPools(config renderConfig) error { return nil } -func (optr *Operator) syncMachineConfigController(config renderConfig) error { +func (optr *Operator) syncMachineConfigController(config *renderConfig) error { crBytes, err := renderAsset(config, "manifests/machineconfigcontroller/clusterrole.yaml") if err != nil { return err @@ -164,16 +164,14 @@ func (optr *Operator) syncMachineConfigController(config renderConfig) error { return err } if updated { - var waitErrs []error - waitErrs = append(waitErrs, optr.waitForDeploymentRollout(mcc)) - waitErrs = append(waitErrs, optr.waitForControllerConfigToBeCompleted(cc)) + waitErrs := []error{optr.waitForDeploymentRollout(mcc), optr.waitForControllerConfigToBeCompleted(cc)} agg := utilerrors.NewAggregate(waitErrs) return agg } return nil } -func (optr *Operator) syncMachineConfigDaemon(config renderConfig) error { +func (optr *Operator) syncMachineConfigDaemon(config *renderConfig) error { for _, path := range []string{ "manifests/machineconfigdaemon/clusterrole.yaml", "manifests/machineconfigdaemon/events-clusterrole.yaml", @@ -240,7 +238,7 @@ func (optr *Operator) syncMachineConfigDaemon(config renderConfig) error { return nil } -func (optr *Operator) syncMachineConfigServer(config renderConfig) error { +func (optr *Operator) syncMachineConfigServer(config *renderConfig) error { crBytes, err := renderAsset(config, "manifests/machineconfigserver/clusterrole.yaml") if err != nil { return err @@ -313,7 +311,7 @@ func (optr *Operator) syncMachineConfigServer(config renderConfig) error { // syncRequiredMachineConfigPools ensures that all the nodes in machineconfigpools labeled with requiredForUpgradeMachineConfigPoolLabelKey // have updated to the latest configuration. -func (optr *Operator) syncRequiredMachineConfigPools(config renderConfig) error { +func (optr *Operator) syncRequiredMachineConfigPools(_ *renderConfig) error { sel, err := metav1.LabelSelectorAsSelector(metav1.AddLabelToSelector(&metav1.LabelSelector{}, requiredForUpgradeMachineConfigPoolLabelKey, "")) if err != nil { return err diff --git a/pkg/server/api.go b/pkg/server/api.go index 22a1b37c43..3c15b047b4 100644 --- a/pkg/server/api.go +++ b/pkg/server/api.go @@ -103,7 +103,7 @@ func (sh *APIHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { glog.Errorf("couldn't get config for req: %v, error: %v", cr, err) return } - if conf == nil && err == nil { + if conf == nil { w.Header().Set("Content-Length", "0") w.WriteHeader(http.StatusNotFound) return diff --git a/pkg/server/bootstrap_server.go b/pkg/server/bootstrap_server.go index e91c191c81..c0ac28dd26 100644 --- a/pkg/server/bootstrap_server.go +++ b/pkg/server/bootstrap_server.go @@ -11,7 +11,7 @@ import ( "github.com/golang/glog" clientcmd "k8s.io/client-go/tools/clientcmd/api/v1" - "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" + v1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" ) // ensure bootstrapServer implements the @@ -96,7 +96,7 @@ func (bsc *bootstrapServer) GetConfig(cr poolRequest) (*igntypes.Config, error) return nil, fmt.Errorf("server: could not unmarshal file %s, err: %v", fileName, err) } - appenders := getAppenders(cr, currConf, bsc.kubeconfigFunc, mc.Spec.OSImageURL) + appenders := getAppenders(currConf, bsc.kubeconfigFunc, mc.Spec.OSImageURL) for _, a := range appenders { if err := a(&mc.Spec.Config); err != nil { return nil, err diff --git a/pkg/server/cluster_server.go b/pkg/server/cluster_server.go index 02250af65e..3f7c9d938d 100644 --- a/pkg/server/cluster_server.go +++ b/pkg/server/cluster_server.go @@ -13,7 +13,7 @@ import ( clientcmd "k8s.io/client-go/tools/clientcmd" clientcmdv1 "k8s.io/client-go/tools/clientcmd/api/v1" - "github.com/openshift/machine-config-operator/pkg/generated/clientset/versioned/typed/machineconfiguration.openshift.io/v1" + v1 "github.com/openshift/machine-config-operator/pkg/generated/clientset/versioned/typed/machineconfiguration.openshift.io/v1" ) const ( @@ -21,6 +21,7 @@ const ( // is running on, instead of using a config passed to it. inClusterConfig = "" + //nolint:gosec bootstrapTokenDir = "/etc/mcs/bootstrap-token" ) @@ -70,7 +71,7 @@ func (cs *clusterServer) GetConfig(cr poolRequest) (*igntypes.Config, error) { return nil, fmt.Errorf("could not fetch config %s, err: %v", currConf, err) } - appenders := getAppenders(cr, currConf, cs.kubeconfigFunc, mc.Spec.OSImageURL) + appenders := getAppenders(currConf, cs.kubeconfigFunc, mc.Spec.OSImageURL) for _, a := range appenders { if err := a(&mc.Spec.Config); err != nil { return nil, err @@ -89,7 +90,7 @@ func getClientConfig(path string) (*rest.Config, error) { return rest.InClusterConfig() } -func kubeconfigFromSecret(secertDir string, apiserverURL string) ([]byte, []byte, error) { +func kubeconfigFromSecret(secertDir, apiserverURL string) ([]byte, []byte, error) { caFile := filepath.Join(secertDir, corev1.ServiceAccountRootCAKey) tokenFile := filepath.Join(secertDir, corev1.ServiceAccountTokenKey) caData, err := ioutil.ReadFile(caFile) diff --git a/pkg/server/server.go b/pkg/server/server.go index 7feddc7015..e8d43fad78 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -3,7 +3,6 @@ package server import ( "encoding/json" "fmt" - "io/ioutil" "net/url" igntypes "github.com/coreos/ignition/config/v2_2/types" @@ -16,9 +15,6 @@ const ( // of the KubeConfig file on the machine. defaultMachineKubeConfPath = "/etc/kubernetes/kubeconfig" - // From https://github.com/openshift/pivot/pull/25/commits/c77788a35d7ee4058d1410e89e6c7937bca89f6c#diff-04c6e90faac2675aa89e2176d2eec7d8R44 - pivotRebootNeeded = "/run/pivot/reboot-needed" - // defaultFileSystem defines the default file system to be // used for writing the ignition files created by the // server. @@ -37,7 +33,7 @@ type Server interface { GetConfig(poolRequest) (*igntypes.Config, error) } -func getAppenders(cr poolRequest, currMachineConfig string, f kubeconfigFunc, osimageurl string) []appenderFunc { +func getAppenders(currMachineConfig string, f kubeconfigFunc, osimageurl string) []appenderFunc { appenders := []appenderFunc{ // append machine annotations file. func(config *igntypes.Config) error { return appendNodeAnnotations(config, currMachineConfig) }, @@ -96,7 +92,7 @@ func appendNodeAnnotations(conf *igntypes.Config, currConf string) error { if err != nil { return err } - appendFileToIgnition(conf, daemonconsts.InitialNodeAnnotationsFilePath, string(anno)) + appendFileToIgnition(conf, daemonconsts.InitialNodeAnnotationsFilePath, anno) return nil } @@ -113,15 +109,6 @@ func getNodeAnnotation(conf string) (string, error) { return string(contents), nil } -func copyFileToIgnition(conf *igntypes.Config, outPath, srcPath string) error { - contents, err := ioutil.ReadFile(srcPath) - if err != nil { - return fmt.Errorf("could not read file from: %s, err: %v", srcPath, err) - } - appendFileToIgnition(conf, outPath, string(contents)) - return nil -} - func appendFileToIgnition(conf *igntypes.Config, outPath, contents string) { fileMode := int(420) file := igntypes.File{ diff --git a/pkg/server/server_test.go b/pkg/server/server_test.go index 0436c640c0..560f83ae0c 100644 --- a/pkg/server/server_test.go +++ b/pkg/server/server_test.go @@ -10,7 +10,7 @@ import ( igntypes "github.com/coreos/ignition/config/v2_2/types" yaml "github.com/ghodss/yaml" - "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" + v1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" daemonconsts "github.com/openshift/machine-config-operator/pkg/daemon/constants" "github.com/openshift/machine-config-operator/pkg/generated/clientset/versioned/fake" ) diff --git a/test/e2e/mcd_test.go b/test/e2e/mcd_test.go index f3aaf74dc0..303721d876 100644 --- a/test/e2e/mcd_test.go +++ b/test/e2e/mcd_test.go @@ -9,6 +9,10 @@ import ( "time" igntypes "github.com/coreos/ignition/config/v2_2/types" + mcv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" + ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" + "github.com/openshift/machine-config-operator/pkg/daemon/constants" + "github.com/openshift/machine-config-operator/test/e2e/framework" "github.com/pkg/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -21,11 +25,6 @@ import ( "k8s.io/apimachinery/pkg/util/jsonmergepatch" "k8s.io/apimachinery/pkg/util/uuid" "k8s.io/apimachinery/pkg/util/wait" - - mcv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" - ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" - "github.com/openshift/machine-config-operator/pkg/daemon/constants" - "github.com/openshift/machine-config-operator/test/e2e/framework" ) // Test case for https://github.com/openshift/machine-config-operator/issues/358 diff --git a/test/helpers/helpers.go b/test/helpers/helpers.go index f44897fca2..b95e004f17 100644 --- a/test/helpers/helpers.go +++ b/test/helpers/helpers.go @@ -43,7 +43,7 @@ func NewMachineConfig(name string, labels map[string]string, osurl string, files } } -func NewMachineConfigPool(name string, mcSelector *metav1.LabelSelector, nodeSelector *metav1.LabelSelector, currentMachineConfig string) *mcfgv1.MachineConfigPool { +func NewMachineConfigPool(name string, mcSelector, nodeSelector *metav1.LabelSelector, currentMachineConfig string) *mcfgv1.MachineConfigPool { return &mcfgv1.MachineConfigPool{ TypeMeta: metav1.TypeMeta{ APIVersion: mcfgv1.SchemeGroupVersion.String(),