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/play.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'pkg/domain/infra/abi/play.go') 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 -- 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/abi/play.go') 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