From 51fbf3da9ee34a8143df5baeda6032c1747446d2 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 25 Apr 2022 15:15:52 +0200 Subject: enable gocritic linter The linter ensures a common code style. - use switch/case instead of else if - use if instead of switch/case for single case statement - add space between comment and text - detect the use of defer with os.Exit() - use short form var += "..." instead of var = var + "..." - detect problems with append() ``` newSlice := append(orgSlice, val) ``` This could lead to nasty bugs because the orgSlice will be changed in place if it has enough capacity too hold the new elements. Thus we newSlice might not be a copy. Of course most of the changes are just cosmetic and do not cause any logic errors but I think it is a good idea to enforce a common style. This should help maintainability. Signed-off-by: Paul Holzinger --- pkg/domain/infra/abi/images.go | 15 +++++++++++---- pkg/domain/infra/abi/play.go | 4 ++-- pkg/domain/infra/abi/system.go | 6 +++--- pkg/domain/infra/abi/trust.go | 6 +++--- 4 files changed, 19 insertions(+), 12 deletions(-) (limited to 'pkg/domain/infra') diff --git a/pkg/domain/infra/abi/images.go b/pkg/domain/infra/abi/images.go index 74478b26d..c3ec7dd8a 100644 --- a/pkg/domain/infra/abi/images.go +++ b/pkg/domain/infra/abi/images.go @@ -785,12 +785,19 @@ func transferRootless(source entities.ImageScpOptions, dest entities.ImageScpOpt return cmdLoad.Run() } -// TransferRootful creates new podman processes using exec.Command and a new uid/gid alongside a cleared environment +// transferRootful creates new podman processes using exec.Command and a new uid/gid alongside a cleared environment func transferRootful(source entities.ImageScpOptions, dest entities.ImageScpOptions, podman string, parentFlags []string) error { - basicCommand := []string{podman} + basicCommand := make([]string, 0, len(parentFlags)+1) + basicCommand = append(basicCommand, podman) basicCommand = append(basicCommand, parentFlags...) - saveCommand := append(basicCommand, "save") - loadCommand := append(basicCommand, "load") + + saveCommand := make([]string, 0, len(basicCommand)+4) + saveCommand = append(saveCommand, basicCommand...) + saveCommand = append(saveCommand, "save") + + loadCommand := make([]string, 0, len(basicCommand)+3) + loadCommand = append(loadCommand, basicCommand...) + loadCommand = append(loadCommand, "load") if source.Quiet { saveCommand = append(saveCommand, "-q") loadCommand = append(loadCommand, "-q") diff --git a/pkg/domain/infra/abi/play.go b/pkg/domain/infra/abi/play.go index 1d347ed8c..f914a53d0 100644 --- a/pkg/domain/infra/abi/play.go +++ b/pkg/domain/infra/abi/play.go @@ -435,7 +435,7 @@ func (ic *ContainerEngine) playKubePod(ctx context.Context, podName string, podY initContainers = append(initContainers, ctr) } for _, container := range podYAML.Spec.Containers { - if !strings.Contains("infra", container.Name) { + if !strings.Contains(container.Name, "infra") { // Error out if the same name is used for more than one container if _, ok := ctrNames[container.Name]; ok { return nil, errors.Errorf("the pod %q is invalid; duplicate container name %q detected", podName, container.Name) @@ -770,7 +770,7 @@ func getBuildFile(imageName string, cwd string) (string, error) { logrus.Error(err.Error()) } - _, err = os.Stat(filepath.Join(dockerfilePath)) + _, err = os.Stat(dockerfilePath) if err == nil { logrus.Debugf("Building %s with %s", imageName, dockerfilePath) return dockerfilePath, nil diff --git a/pkg/domain/infra/abi/system.go b/pkg/domain/infra/abi/system.go index 4361821d5..8e96e4154 100644 --- a/pkg/domain/infra/abi/system.go +++ b/pkg/domain/infra/abi/system.go @@ -150,7 +150,7 @@ func (ic *ContainerEngine) SystemPrune(ctx context.Context, options entities.Sys if err != nil { return nil, err } - reclaimedSpace = reclaimedSpace + reports.PruneReportsSize(containerPruneReports) + reclaimedSpace += reports.PruneReportsSize(containerPruneReports) systemPruneReport.ContainerPruneReports = append(systemPruneReport.ContainerPruneReports, containerPruneReports...) imagePruneOptions := entities.ImagePruneOptions{ All: options.All, @@ -158,7 +158,7 @@ func (ic *ContainerEngine) SystemPrune(ctx context.Context, options entities.Sys } imageEngine := ImageEngine{Libpod: ic.Libpod} imagePruneReports, err := imageEngine.Prune(ctx, imagePruneOptions) - reclaimedSpace = reclaimedSpace + reports.PruneReportsSize(imagePruneReports) + reclaimedSpace += reports.PruneReportsSize(imagePruneReports) if err != nil { return nil, err @@ -178,7 +178,7 @@ func (ic *ContainerEngine) SystemPrune(ctx context.Context, options entities.Sys if len(volumePruneReport) > 0 { found = true } - reclaimedSpace = reclaimedSpace + reports.PruneReportsSize(volumePruneReport) + reclaimedSpace += reports.PruneReportsSize(volumePruneReport) systemPruneReport.VolumePruneReports = append(systemPruneReport.VolumePruneReports, volumePruneReport...) } } diff --git a/pkg/domain/infra/abi/trust.go b/pkg/domain/infra/abi/trust.go index d53fe16d1..58f099bb6 100644 --- a/pkg/domain/infra/abi/trust.go +++ b/pkg/domain/infra/abi/trust.go @@ -142,15 +142,15 @@ func getPolicyShowOutput(policyContentStruct trust.PolicyContent, systemRegistri Type: trustTypeDescription(repoval[0].Type), } // TODO - keyarr is not used and I don't know its intent; commenting out for now for someone to fix later - //keyarr := []string{} + // keyarr := []string{} uids := []string{} for _, repoele := range repoval { if len(repoele.KeyPath) > 0 { - //keyarr = append(keyarr, repoele.KeyPath) + // keyarr = append(keyarr, repoele.KeyPath) uids = append(uids, trust.GetGPGIdFromKeyPath(repoele.KeyPath)...) } if len(repoele.KeyData) > 0 { - //keyarr = append(keyarr, string(repoele.KeyData)) + // keyarr = append(keyarr, string(repoele.KeyData)) uids = append(uids, trust.GetGPGIdFromKeyData(repoele.KeyData)...) } } -- cgit v1.2.3-54-g00ecf From 4f8ece76fff31d31570af56e0ec4a4092e015b33 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Tue, 26 Apr 2022 16:53:08 +0200 Subject: play kube: do not skip containers by name We should not exclude contianers by name. If a users has a container with the name "inf" it is currently skipped. This is wrong. The k8s yaml does not contain infra containers so we do not have to skip them. Signed-off-by: Paul Holzinger --- pkg/domain/infra/abi/play.go | 86 ++++++++++++++++++++++---------------------- test/e2e/play_kube_test.go | 2 +- 2 files changed, 43 insertions(+), 45 deletions(-) (limited to 'pkg/domain/infra') diff --git a/pkg/domain/infra/abi/play.go b/pkg/domain/infra/abi/play.go index f914a53d0..45500b23c 100644 --- a/pkg/domain/infra/abi/play.go +++ b/pkg/domain/infra/abi/play.go @@ -435,53 +435,51 @@ func (ic *ContainerEngine) playKubePod(ctx context.Context, podName string, podY initContainers = append(initContainers, ctr) } for _, container := range podYAML.Spec.Containers { - if !strings.Contains(container.Name, "infra") { - // Error out if the same name is used for more than one container - if _, ok := ctrNames[container.Name]; ok { - return nil, errors.Errorf("the pod %q is invalid; duplicate container name %q detected", podName, container.Name) - } - ctrNames[container.Name] = "" - pulledImage, labels, err := ic.getImageAndLabelInfo(ctx, cwd, annotations, writer, container, options) - if err != nil { - return nil, err - } + // Error out if the same name is used for more than one container + if _, ok := ctrNames[container.Name]; ok { + return nil, errors.Errorf("the pod %q is invalid; duplicate container name %q detected", podName, container.Name) + } + ctrNames[container.Name] = "" + pulledImage, labels, err := ic.getImageAndLabelInfo(ctx, cwd, annotations, writer, container, options) + if err != nil { + return nil, err + } - for k, v := range podSpec.PodSpecGen.Labels { // add podYAML labels - labels[k] = v - } + for k, v := range podSpec.PodSpecGen.Labels { // add podYAML labels + labels[k] = v + } - specgenOpts := kube.CtrSpecGenOptions{ - Annotations: annotations, - Container: container, - Image: pulledImage, - Volumes: volumes, - PodID: pod.ID(), - PodName: podName, - PodInfraID: podInfraID, - ConfigMaps: configMaps, - SeccompPaths: seccompPaths, - RestartPolicy: ctrRestartPolicy, - NetNSIsHost: p.NetNS.IsHost(), - SecretsManager: secretsManager, - LogDriver: options.LogDriver, - LogOptions: options.LogOptions, - Labels: labels, - } - specGen, err := kube.ToSpecGen(ctx, &specgenOpts) - if err != nil { - return nil, err - } - specGen.RawImageName = container.Image - rtSpec, spec, opts, err := generate.MakeContainer(ctx, ic.Libpod, specGen, false, nil) - if err != nil { - return nil, err - } - ctr, err := generate.ExecuteCreate(ctx, ic.Libpod, rtSpec, spec, false, opts...) - if err != nil { - return nil, err - } - containers = append(containers, ctr) + specgenOpts := kube.CtrSpecGenOptions{ + Annotations: annotations, + Container: container, + Image: pulledImage, + Volumes: volumes, + PodID: pod.ID(), + PodName: podName, + PodInfraID: podInfraID, + ConfigMaps: configMaps, + SeccompPaths: seccompPaths, + RestartPolicy: ctrRestartPolicy, + NetNSIsHost: p.NetNS.IsHost(), + SecretsManager: secretsManager, + LogDriver: options.LogDriver, + LogOptions: options.LogOptions, + Labels: labels, + } + specGen, err := kube.ToSpecGen(ctx, &specgenOpts) + if err != nil { + return nil, err + } + specGen.RawImageName = container.Image + rtSpec, spec, opts, err := generate.MakeContainer(ctx, ic.Libpod, specGen, false, nil) + if err != nil { + return nil, err + } + ctr, err := generate.ExecuteCreate(ctx, ic.Libpod, rtSpec, spec, false, opts...) + if err != nil { + return nil, err } + containers = append(containers, ctr) } if options.Start != types.OptionalBoolFalse { diff --git a/test/e2e/play_kube_test.go b/test/e2e/play_kube_test.go index 4dd05c755..6d89a844e 100644 --- a/test/e2e/play_kube_test.go +++ b/test/e2e/play_kube_test.go @@ -2995,7 +2995,7 @@ invalid kube kind It("podman play kube with auto update annotations for all containers", func() { ctr01Name := "ctr01" - ctr02Name := "ctr02" + ctr02Name := "infra" podName := "foo" autoUpdateRegistry := "io.containers.autoupdate" autoUpdateRegistryValue := "registry" -- cgit v1.2.3-54-g00ecf