diff --git a/pkg/cri/server/container_create_linux_test.go b/pkg/cri/server/container_create_linux_test.go index e0a7ea998..639713fd6 100644 --- a/pkg/cri/server/container_create_linux_test.go +++ b/pkg/cri/server/container_create_linux_test.go @@ -199,12 +199,14 @@ func TestContainerCapabilities(t *testing.T) { testContainerName := "container-name" testPid := uint32(1234) allCaps := cap.Known() - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string capability *runtime.Capability includes []string excludes []string }{ - "should be able to add/drop capabilities": { + { + desc: "should be able to add/drop capabilities", capability: &runtime.Capability{ AddCapabilities: []string{"SYS_ADMIN"}, DropCapabilities: []string{"CHOWN"}, @@ -212,19 +214,22 @@ func TestContainerCapabilities(t *testing.T) { includes: []string{"CAP_SYS_ADMIN"}, excludes: []string{"CAP_CHOWN"}, }, - "should be able to add all capabilities": { + { + desc: "should be able to add all capabilities", capability: &runtime.Capability{ AddCapabilities: []string{"ALL"}, }, includes: allCaps, }, - "should be able to drop all capabilities": { + { + desc: "should be able to drop all capabilities", capability: &runtime.Capability{ DropCapabilities: []string{"ALL"}, }, excludes: allCaps, }, - "should be able to drop capabilities with add all": { + { + desc: "should be able to drop capabilities with add all", capability: &runtime.Capability{ AddCapabilities: []string{"ALL"}, DropCapabilities: []string{"CHOWN"}, @@ -232,7 +237,8 @@ func TestContainerCapabilities(t *testing.T) { includes: util.SubtractStringSlice(allCaps, "CAP_CHOWN"), excludes: []string{"CAP_CHOWN"}, }, - "should be able to add capabilities with drop all": { + { + desc: "should be able to add capabilities with drop all", capability: &runtime.Capability{ AddCapabilities: []string{"SYS_ADMIN"}, DropCapabilities: []string{"ALL"}, @@ -241,7 +247,8 @@ func TestContainerCapabilities(t *testing.T) { excludes: util.SubtractStringSlice(allCaps, "CAP_SYS_ADMIN"), }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { containerConfig, sandboxConfig, imageConfig, specCheck := getCreateContainerTestData() ociRuntime := config.Runtime{} c := newTestCRIService() @@ -403,33 +410,39 @@ func TestContainerAndSandboxPrivileged(t *testing.T) { containerConfig, sandboxConfig, imageConfig, _ := getCreateContainerTestData() ociRuntime := config.Runtime{} c := newTestCRIService() - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string containerPrivileged bool sandboxPrivileged bool expectError bool }{ - "privileged container in non-privileged sandbox should fail": { + { + desc: "privileged container in non-privileged sandbox should fail", containerPrivileged: true, sandboxPrivileged: false, expectError: true, }, - "privileged container in privileged sandbox should be fine": { + { + desc: "privileged container in privileged sandbox should be fine", containerPrivileged: true, sandboxPrivileged: true, expectError: false, }, - "non-privileged container in privileged sandbox should be fine": { + { + desc: "non-privileged container in privileged sandbox should be fine", containerPrivileged: false, sandboxPrivileged: true, expectError: false, }, - "non-privileged container in non-privileged sandbox should be fine": { + { + desc: "non-privileged container in non-privileged sandbox should be fine", containerPrivileged: false, sandboxPrivileged: false, expectError: false, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { containerConfig.Linux.SecurityContext.Privileged = test.containerPrivileged sandboxConfig.Linux.SecurityContext = &runtime.LinuxSandboxSecurityContext{ Privileged: test.sandboxPrivileged, @@ -446,13 +459,15 @@ func TestContainerAndSandboxPrivileged(t *testing.T) { func TestContainerMounts(t *testing.T) { const testSandboxID = "test-id" - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string statFn func(string) (os.FileInfo, error) criMounts []*runtime.Mount securityContext *runtime.LinuxContainerSecurityContext expectedMounts []*runtime.Mount }{ - "should setup ro mount when rootfs is read-only": { + { + desc: "should setup ro mount when rootfs is read-only", securityContext: &runtime.LinuxContainerSecurityContext{ ReadonlyRootfs: true, }, @@ -483,7 +498,8 @@ func TestContainerMounts(t *testing.T) { }, }, }, - "should setup rw mount when rootfs is read-write": { + { + desc: "should setup rw mount when rootfs is read-write", securityContext: &runtime.LinuxContainerSecurityContext{}, expectedMounts: []*runtime.Mount{ { @@ -512,7 +528,8 @@ func TestContainerMounts(t *testing.T) { }, }, }, - "should use host /dev/shm when host ipc is set": { + { + desc: "should use host /dev/shm when host ipc is set", securityContext: &runtime.LinuxContainerSecurityContext{ NamespaceOptions: &runtime.NamespaceOption{Ipc: runtime.NamespaceMode_NODE}, }, @@ -542,7 +559,8 @@ func TestContainerMounts(t *testing.T) { }, }, }, - "should skip container mounts if already mounted by CRI": { + { + desc: "should skip container mounts if already mounted by CRI", criMounts: []*runtime.Mount{ { ContainerPath: "/etc/hostname", @@ -564,7 +582,8 @@ func TestContainerMounts(t *testing.T) { securityContext: &runtime.LinuxContainerSecurityContext{}, expectedMounts: nil, }, - "should skip hostname mount if the old sandbox doesn't have hostname file": { + { + desc: "should skip hostname mount if the old sandbox doesn't have hostname file", statFn: func(path string) (os.FileInfo, error) { assert.Equal(t, filepath.Join(testRootDir, sandboxesDir, testSandboxID, "hostname"), path) return nil, errors.New("random error") @@ -592,7 +611,8 @@ func TestContainerMounts(t *testing.T) { }, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { config := &runtime.ContainerConfig{ Metadata: &runtime.ContainerMetadata{ Name: "test-name", @@ -606,7 +626,7 @@ func TestContainerMounts(t *testing.T) { c := newTestCRIService() c.os.(*ostesting.FakeOS).StatFn = test.statFn mounts := c.containerMounts(testSandboxID, config) - assert.Equal(t, test.expectedMounts, mounts, desc) + assert.Equal(t, test.expectedMounts, mounts, test.desc) }) } } @@ -619,22 +639,26 @@ func TestPrivilegedBindMount(t *testing.T) { containerConfig, sandboxConfig, imageConfig, _ := getCreateContainerTestData() ociRuntime := config.Runtime{} - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string privileged bool expectedSysFSRO bool expectedCgroupFSRO bool }{ - "sysfs and cgroupfs should mount as 'ro' by default": { + { + desc: "sysfs and cgroupfs should mount as 'ro' by default", expectedSysFSRO: true, expectedCgroupFSRO: true, }, - "sysfs and cgroupfs should not mount as 'ro' if privileged": { + { + desc: "sysfs and cgroupfs should not mount as 'ro' if privileged", privileged: true, expectedSysFSRO: false, expectedCgroupFSRO: false, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { containerConfig.Linux.SecurityContext.Privileged = test.privileged sandboxConfig.Linux.SecurityContext.Privileged = test.privileged @@ -678,13 +702,15 @@ func TestMountPropagation(t *testing.T) { }, nil } - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string criMount *runtime.Mount fakeLookupMountFn func(string) (mount.Info, error) optionsCheck []string expectErr bool }{ - "HostPath should mount as 'rprivate' if propagation is MountPropagation_PROPAGATION_PRIVATE": { + { + desc: "HostPath should mount as 'rprivate' if propagation is MountPropagation_PROPAGATION_PRIVATE", criMount: &runtime.Mount{ ContainerPath: "container-path", HostPath: "host-path", @@ -694,7 +720,8 @@ func TestMountPropagation(t *testing.T) { optionsCheck: []string{"rbind", "rprivate"}, expectErr: false, }, - "HostPath should mount as 'rslave' if propagation is MountPropagation_PROPAGATION_HOST_TO_CONTAINER": { + { + desc: "HostPath should mount as 'rslave' if propagation is MountPropagation_PROPAGATION_HOST_TO_CONTAINER", criMount: &runtime.Mount{ ContainerPath: "container-path", HostPath: "host-path", @@ -704,7 +731,8 @@ func TestMountPropagation(t *testing.T) { optionsCheck: []string{"rbind", "rslave"}, expectErr: false, }, - "HostPath should mount as 'rshared' if propagation is MountPropagation_PROPAGATION_BIDIRECTIONAL": { + { + desc: "HostPath should mount as 'rshared' if propagation is MountPropagation_PROPAGATION_BIDIRECTIONAL", criMount: &runtime.Mount{ ContainerPath: "container-path", HostPath: "host-path", @@ -714,7 +742,8 @@ func TestMountPropagation(t *testing.T) { optionsCheck: []string{"rbind", "rshared"}, expectErr: false, }, - "HostPath should mount as 'rprivate' if propagation is illegal": { + { + desc: "HostPath should mount as 'rprivate' if propagation is illegal", criMount: &runtime.Mount{ ContainerPath: "container-path", HostPath: "host-path", @@ -724,7 +753,8 @@ func TestMountPropagation(t *testing.T) { optionsCheck: []string{"rbind", "rprivate"}, expectErr: false, }, - "Expect an error if HostPath isn't shared and mount propagation is MountPropagation_PROPAGATION_BIDIRECTIONAL": { + { + desc: "Expect an error if HostPath isn't shared and mount propagation is MountPropagation_PROPAGATION_BIDIRECTIONAL", criMount: &runtime.Mount{ ContainerPath: "container-path", HostPath: "host-path", @@ -733,7 +763,8 @@ func TestMountPropagation(t *testing.T) { fakeLookupMountFn: slaveLookupMountFn, expectErr: true, }, - "Expect an error if HostPath isn't slave or shared and mount propagation is MountPropagation_PROPAGATION_HOST_TO_CONTAINER": { + { + desc: "Expect an error if HostPath isn't slave or shared and mount propagation is MountPropagation_PROPAGATION_HOST_TO_CONTAINER", criMount: &runtime.Mount{ ContainerPath: "container-path", HostPath: "host-path", @@ -743,7 +774,8 @@ func TestMountPropagation(t *testing.T) { expectErr: true, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { c := newTestCRIService() c.os.(*ostesting.FakeOS).LookupMountFn = test.fakeLookupMountFn config, _, _, _ := getCreateContainerTestData() @@ -770,24 +802,28 @@ func TestPidNamespace(t *testing.T) { containerConfig, sandboxConfig, imageConfig, _ := getCreateContainerTestData() ociRuntime := config.Runtime{} c := newTestCRIService() - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string pidNS runtime.NamespaceMode expected runtimespec.LinuxNamespace }{ - "node namespace mode": { + { + desc: "node namespace mode", pidNS: runtime.NamespaceMode_NODE, expected: runtimespec.LinuxNamespace{ Type: runtimespec.PIDNamespace, Path: opts.GetPIDNamespace(testPid), }, }, - "container namespace mode": { + { + desc: "container namespace mode", pidNS: runtime.NamespaceMode_CONTAINER, expected: runtimespec.LinuxNamespace{ Type: runtimespec.PIDNamespace, }, }, - "pod namespace mode": { + { + desc: "pod namespace mode", pidNS: runtime.NamespaceMode_POD, expected: runtimespec.LinuxNamespace{ Type: runtimespec.PIDNamespace, @@ -795,7 +831,8 @@ func TestPidNamespace(t *testing.T) { }, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { containerConfig.Linux.SecurityContext.NamespaceOptions = &runtime.NamespaceOption{Pid: test.pidNS} spec, err := c.containerSpec(testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime) require.NoError(t, err) @@ -827,7 +864,8 @@ func TestUserNamespace(t *testing.T) { containerConfig, sandboxConfig, imageConfig, _ := getCreateContainerTestData() ociRuntime := config.Runtime{} c := newTestCRIService() - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string userNS *runtime.UserNamespace sandboxUserNS *runtime.UserNamespace expNS *runtimespec.LinuxNamespace @@ -836,7 +874,8 @@ func TestUserNamespace(t *testing.T) { expGIDMapping []runtimespec.LinuxIDMapping err bool }{ - "node namespace mode": { + { + desc: "node namespace mode", userNS: &runtime.UserNamespace{Mode: runtime.NamespaceMode_NODE}, // Expect userns to NOT be present. expNotNS: &runtimespec.LinuxNamespace{ @@ -844,7 +883,8 @@ func TestUserNamespace(t *testing.T) { Path: opts.GetUserNamespace(testPid), }, }, - "node namespace mode with mappings": { + { + desc: "node namespace mode with mappings", userNS: &runtime.UserNamespace{ Mode: runtime.NamespaceMode_NODE, Uids: []*runtime.IDMapping{&idMap}, @@ -852,19 +892,23 @@ func TestUserNamespace(t *testing.T) { }, err: true, }, - "container namespace mode": { + { + desc: "container namespace mode", userNS: &runtime.UserNamespace{Mode: runtime.NamespaceMode_CONTAINER}, err: true, }, - "target namespace mode": { + { + desc: "target namespace mode", userNS: &runtime.UserNamespace{Mode: runtime.NamespaceMode_TARGET}, err: true, }, - "unknown namespace mode": { + { + desc: "unknown namespace mode", userNS: &runtime.UserNamespace{Mode: runtime.NamespaceMode(100)}, err: true, }, - "pod namespace mode": { + { + desc: "pod namespace mode", userNS: &runtime.UserNamespace{ Mode: runtime.NamespaceMode_POD, Uids: []*runtime.IDMapping{&idMap}, @@ -877,7 +921,8 @@ func TestUserNamespace(t *testing.T) { expUIDMapping: []runtimespec.LinuxIDMapping{expIDMap}, expGIDMapping: []runtimespec.LinuxIDMapping{expIDMap}, }, - "pod namespace mode with inconsistent sandbox config (different GIDs)": { + { + desc: "pod namespace mode with inconsistent sandbox config (different GIDs)", userNS: &runtime.UserNamespace{ Mode: runtime.NamespaceMode_POD, Uids: []*runtime.IDMapping{&idMap}, @@ -890,7 +935,8 @@ func TestUserNamespace(t *testing.T) { }, err: true, }, - "pod namespace mode with inconsistent sandbox config (different UIDs)": { + { + desc: "pod namespace mode with inconsistent sandbox config (different UIDs)", userNS: &runtime.UserNamespace{ Mode: runtime.NamespaceMode_POD, Uids: []*runtime.IDMapping{&idMap}, @@ -903,7 +949,8 @@ func TestUserNamespace(t *testing.T) { }, err: true, }, - "pod namespace mode with inconsistent sandbox config (different len)": { + { + desc: "pod namespace mode with inconsistent sandbox config (different len)", userNS: &runtime.UserNamespace{ Mode: runtime.NamespaceMode_POD, Uids: []*runtime.IDMapping{&idMap}, @@ -916,7 +963,8 @@ func TestUserNamespace(t *testing.T) { }, err: true, }, - "pod namespace mode with inconsistent sandbox config (different mode)": { + { + desc: "pod namespace mode with inconsistent sandbox config (different mode)", userNS: &runtime.UserNamespace{ Mode: runtime.NamespaceMode_POD, Uids: []*runtime.IDMapping{&idMap}, @@ -929,7 +977,8 @@ func TestUserNamespace(t *testing.T) { }, err: true, }, - "pod namespace mode with several mappings": { + { + desc: "pod namespace mode with several mappings", userNS: &runtime.UserNamespace{ Mode: runtime.NamespaceMode_POD, Uids: []*runtime.IDMapping{&idMap, &idMap}, @@ -937,7 +986,8 @@ func TestUserNamespace(t *testing.T) { }, err: true, }, - "pod namespace mode with uneven mappings": { + { + desc: "pod namespace mode with uneven mappings", userNS: &runtime.UserNamespace{ Mode: runtime.NamespaceMode_POD, Uids: []*runtime.IDMapping{&idMap, &idMap}, @@ -946,9 +996,8 @@ func TestUserNamespace(t *testing.T) { err: true, }, } { - desc := desc test := test - t.Run(desc, func(t *testing.T) { + t.Run(test.desc, func(t *testing.T) { containerConfig.Linux.SecurityContext.NamespaceOptions = &runtime.NamespaceOption{UsernsOptions: test.userNS} // By default, set sandbox and container config to the same (this is // required by containerSpec). However, if the test wants to test for what @@ -997,7 +1046,8 @@ func TestNoDefaultRunMount(t *testing.T) { } func TestGenerateSeccompSecurityProfileSpecOpts(t *testing.T) { - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string profile string privileged bool disable bool @@ -1006,98 +1056,118 @@ func TestGenerateSeccompSecurityProfileSpecOpts(t *testing.T) { defaultProfile string sp *runtime.SecurityProfile }{ - "should return error if seccomp is specified when seccomp is not supported": { + { + desc: "should return error if seccomp is specified when seccomp is not supported", profile: runtimeDefault, disable: true, expectErr: true, }, - "should not return error if seccomp is not specified when seccomp is not supported": { + { + desc: "should not return error if seccomp is not specified when seccomp is not supported", profile: "", disable: true, }, - "should not return error if seccomp is unconfined when seccomp is not supported": { + { + desc: "should not return error if seccomp is unconfined when seccomp is not supported", profile: unconfinedProfile, disable: true, }, - "should not set seccomp when privileged is true": { + { + desc: "should not set seccomp when privileged is true", profile: seccompDefaultProfile, privileged: true, }, - "should not set seccomp when seccomp is unconfined": { + { + desc: "should not set seccomp when seccomp is unconfined", profile: unconfinedProfile, }, - "should not set seccomp when seccomp is not specified": { + { + desc: "should not set seccomp when seccomp is not specified", profile: "", }, - "should set default seccomp when seccomp is runtime/default": { + { + desc: "should set default seccomp when seccomp is runtime/default", profile: runtimeDefault, specOpts: seccomp.WithDefaultProfile(), }, - "should set default seccomp when seccomp is docker/default": { + { + desc: "should set default seccomp when seccomp is docker/default", profile: dockerDefault, specOpts: seccomp.WithDefaultProfile(), }, - "should set specified profile when local profile is specified": { + { + desc: "should set specified profile when local profile is specified", profile: profileNamePrefix + "test-profile", specOpts: seccomp.WithProfile("test-profile"), }, - "should use default profile when seccomp is empty": { + { + desc: "should use default profile when seccomp is empty", defaultProfile: profileNamePrefix + "test-profile", specOpts: seccomp.WithProfile("test-profile"), }, - "should fallback to docker/default when seccomp is empty and default is runtime/default": { + { + desc: "should fallback to docker/default when seccomp is empty and default is runtime/default", defaultProfile: runtimeDefault, specOpts: seccomp.WithDefaultProfile(), }, //----------------------------------------------- // now buckets for the SecurityProfile variants //----------------------------------------------- - "sp should return error if seccomp is specified when seccomp is not supported": { + { + desc: "sp should return error if seccomp is specified when seccomp is not supported", disable: true, expectErr: true, sp: &runtime.SecurityProfile{ ProfileType: runtime.SecurityProfile_RuntimeDefault, }, }, - "sp should not return error if seccomp is unconfined when seccomp is not supported": { + { + desc: "sp should not return error if seccomp is unconfined when seccomp is not supported", disable: true, sp: &runtime.SecurityProfile{ ProfileType: runtime.SecurityProfile_Unconfined, }, }, - "sp should not set seccomp when privileged is true": { + { + desc: "sp should not set seccomp when privileged is true", privileged: true, sp: &runtime.SecurityProfile{ ProfileType: runtime.SecurityProfile_RuntimeDefault, }, }, - "sp should not set seccomp when seccomp is unconfined": { + { + desc: "sp should not set seccomp when seccomp is unconfined", sp: &runtime.SecurityProfile{ ProfileType: runtime.SecurityProfile_Unconfined, }, }, - "sp should not set seccomp when seccomp is not specified": {}, - "sp should set default seccomp when seccomp is runtime/default": { + { + desc: "sp should not set seccomp when seccomp is not specified"}, + { + desc: "sp should set default seccomp when seccomp is runtime/default", specOpts: seccomp.WithDefaultProfile(), sp: &runtime.SecurityProfile{ ProfileType: runtime.SecurityProfile_RuntimeDefault, }, }, - "sp should set specified profile when local profile is specified": { + { + desc: "sp should set specified profile when local profile is specified", specOpts: seccomp.WithProfile("test-profile"), sp: &runtime.SecurityProfile{ ProfileType: runtime.SecurityProfile_Localhost, LocalhostRef: profileNamePrefix + "test-profile", }, }, - "sp should set specified profile when local profile is specified even without prefix": { + { + desc: "sp should set specified profile when local profile is specified even without prefix", specOpts: seccomp.WithProfile("test-profile"), sp: &runtime.SecurityProfile{ ProfileType: runtime.SecurityProfile_Localhost, LocalhostRef: "test-profile", }, }, - "sp should return error if specified profile is invalid": { + { + desc: "sp should return error if specified profile is invalid", expectErr: true, sp: &runtime.SecurityProfile{ ProfileType: runtime.SecurityProfile_RuntimeDefault, @@ -1105,7 +1175,8 @@ func TestGenerateSeccompSecurityProfileSpecOpts(t *testing.T) { }, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { cri := &criService{} cri.config.UnsetSeccompProfile = test.defaultProfile ssp := test.sp @@ -1137,7 +1208,8 @@ func TestGenerateSeccompSecurityProfileSpecOpts(t *testing.T) { } func TestGenerateApparmorSpecOpts(t *testing.T) { - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string profile string privileged bool disable bool @@ -1145,103 +1217,123 @@ func TestGenerateApparmorSpecOpts(t *testing.T) { expectErr bool sp *runtime.SecurityProfile }{ - "should return error if apparmor is specified when apparmor is not supported": { + { + desc: "should return error if apparmor is specified when apparmor is not supported", profile: runtimeDefault, disable: true, expectErr: true, }, - "should not return error if apparmor is not specified when apparmor is not supported": { + { + desc: "should not return error if apparmor is not specified when apparmor is not supported", profile: "", disable: true, }, - "should set default apparmor when apparmor is not specified": { + { + desc: "should set default apparmor when apparmor is not specified", profile: "", specOpts: apparmor.WithDefaultProfile(appArmorDefaultProfileName), }, - "should not apparmor when apparmor is not specified and privileged is true": { + { + desc: "should not apparmor when apparmor is not specified and privileged is true", profile: "", privileged: true, }, - "should not return error if apparmor is unconfined when apparmor is not supported": { + { + desc: "should not return error if apparmor is unconfined when apparmor is not supported", profile: unconfinedProfile, disable: true, }, - "should not apparmor when apparmor is unconfined": { + { + desc: "should not apparmor when apparmor is unconfined", profile: unconfinedProfile, }, - "should not apparmor when apparmor is unconfined and privileged is true": { + { + desc: "should not apparmor when apparmor is unconfined and privileged is true", profile: unconfinedProfile, privileged: true, }, - "should set default apparmor when apparmor is runtime/default": { + { + desc: "should set default apparmor when apparmor is runtime/default", profile: runtimeDefault, specOpts: apparmor.WithDefaultProfile(appArmorDefaultProfileName), }, - "should not apparmor when apparmor is default and privileged is true": { + { + desc: "should not apparmor when apparmor is default and privileged is true", profile: runtimeDefault, privileged: true, }, // TODO (mikebrow) add success with existing defined profile tests - "should return error when undefined local profile is specified": { + { + desc: "should return error when undefined local profile is specified", profile: profileNamePrefix + "test-profile", expectErr: true, }, - "should return error when undefined local profile is specified and privileged is true": { + { + desc: "should return error when undefined local profile is specified and privileged is true", profile: profileNamePrefix + "test-profile", privileged: true, expectErr: true, }, - "should return error if specified profile is invalid": { + { + desc: "should return error if specified profile is invalid", profile: "test-profile", expectErr: true, }, //-------------------------------------- // buckets for SecurityProfile struct //-------------------------------------- - "sp should return error if apparmor is specified when apparmor is not supported": { + { + desc: "sp should return error if apparmor is specified when apparmor is not supported", disable: true, expectErr: true, sp: &runtime.SecurityProfile{ ProfileType: runtime.SecurityProfile_RuntimeDefault, }, }, - "sp should not return error if apparmor is unconfined when apparmor is not supported": { + { + desc: "sp should not return error if apparmor is unconfined when apparmor is not supported", disable: true, sp: &runtime.SecurityProfile{ ProfileType: runtime.SecurityProfile_Unconfined, }, }, - "sp should not apparmor when apparmor is unconfined": { + { + desc: "sp should not apparmor when apparmor is unconfined", sp: &runtime.SecurityProfile{ ProfileType: runtime.SecurityProfile_Unconfined, }, }, - "sp should not apparmor when apparmor is unconfined and privileged is true": { + { + desc: "sp should not apparmor when apparmor is unconfined and privileged is true", privileged: true, sp: &runtime.SecurityProfile{ ProfileType: runtime.SecurityProfile_Unconfined, }, }, - "sp should set default apparmor when apparmor is runtime/default": { + { + desc: "sp should set default apparmor when apparmor is runtime/default", specOpts: apparmor.WithDefaultProfile(appArmorDefaultProfileName), sp: &runtime.SecurityProfile{ ProfileType: runtime.SecurityProfile_RuntimeDefault, }, }, - "sp should not apparmor when apparmor is default and privileged is true": { + { + desc: "sp should not apparmor when apparmor is default and privileged is true", privileged: true, sp: &runtime.SecurityProfile{ ProfileType: runtime.SecurityProfile_RuntimeDefault, }, }, - "sp should return error when undefined local profile is specified": { + { + desc: "sp should return error when undefined local profile is specified", expectErr: true, sp: &runtime.SecurityProfile{ ProfileType: runtime.SecurityProfile_Localhost, LocalhostRef: profileNamePrefix + "test-profile", }, }, - "sp should return error when undefined local profile is specified even without prefix": { + { + desc: "sp should return error when undefined local profile is specified even without prefix", profile: profileNamePrefix + "test-profile", expectErr: true, sp: &runtime.SecurityProfile{ @@ -1249,7 +1341,8 @@ func TestGenerateApparmorSpecOpts(t *testing.T) { LocalhostRef: "test-profile", }, }, - "sp should return error when undefined local profile is specified and privileged is true": { + { + desc: "sp should return error when undefined local profile is specified and privileged is true", privileged: true, expectErr: true, sp: &runtime.SecurityProfile{ @@ -1258,7 +1351,8 @@ func TestGenerateApparmorSpecOpts(t *testing.T) { }, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { asp := test.sp csp, err := generateApparmorSecurityProfile(test.profile) if err != nil { @@ -1297,7 +1391,8 @@ func TestMaskedAndReadonlyPaths(t *testing.T) { defaultSpec, err := oci.GenerateSpec(ctrdutil.NamespacedContext(), nil, &containers.Container{ID: testID}) require.NoError(t, err) - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string disableProcMount bool masked []string readonly []string @@ -1305,7 +1400,8 @@ func TestMaskedAndReadonlyPaths(t *testing.T) { expectedReadonly []string privileged bool }{ - "should apply default if not specified when disable_proc_mount = true": { + { + desc: "should apply default if not specified when disable_proc_mount = true", disableProcMount: true, masked: nil, readonly: nil, @@ -1313,7 +1409,8 @@ func TestMaskedAndReadonlyPaths(t *testing.T) { expectedReadonly: defaultSpec.Linux.ReadonlyPaths, privileged: false, }, - "should apply default if not specified when disable_proc_mount = false": { + { + desc: "should apply default if not specified when disable_proc_mount = false", disableProcMount: false, masked: nil, readonly: nil, @@ -1321,33 +1418,38 @@ func TestMaskedAndReadonlyPaths(t *testing.T) { expectedReadonly: []string{}, privileged: false, }, - "should be able to specify empty paths": { + { + desc: "should be able to specify empty paths", masked: []string{}, readonly: []string{}, expectedMasked: []string{}, expectedReadonly: []string{}, privileged: false, }, - "should apply CRI specified paths": { + { + desc: "should apply CRI specified paths", masked: []string{"/proc"}, readonly: []string{"/sys"}, expectedMasked: []string{"/proc"}, expectedReadonly: []string{"/sys"}, privileged: false, }, - "default should be nil for privileged": { + { + desc: "default should be nil for privileged", expectedMasked: nil, expectedReadonly: nil, privileged: true, }, - "should be able to specify empty paths, esp. if privileged": { + { + desc: "should be able to specify empty paths, esp. if privileged", masked: []string{}, readonly: []string{}, expectedMasked: nil, expectedReadonly: nil, privileged: true, }, - "should not apply CRI specified paths if privileged": { + { + desc: "should not apply CRI specified paths if privileged", masked: []string{"/proc"}, readonly: []string{"/sys"}, expectedMasked: nil, @@ -1355,7 +1457,8 @@ func TestMaskedAndReadonlyPaths(t *testing.T) { privileged: true, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { c.config.DisableProcMount = test.disableProcMount containerConfig.Linux.SecurityContext.MaskedPaths = test.masked containerConfig.Linux.SecurityContext.ReadonlyPaths = test.readonly @@ -1385,28 +1488,33 @@ func TestHostname(t *testing.T) { c.os.(*ostesting.FakeOS).HostnameFn = func() (string, error) { return "real-hostname", nil } - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string hostname string networkNs runtime.NamespaceMode expectedEnv string }{ - "should add HOSTNAME=sandbox.Hostname for pod network namespace": { + { + desc: "should add HOSTNAME=sandbox.Hostname for pod network namespace", hostname: "test-hostname", networkNs: runtime.NamespaceMode_POD, expectedEnv: "HOSTNAME=test-hostname", }, - "should add HOSTNAME=sandbox.Hostname for host network namespace": { + { + desc: "should add HOSTNAME=sandbox.Hostname for host network namespace", hostname: "test-hostname", networkNs: runtime.NamespaceMode_NODE, expectedEnv: "HOSTNAME=test-hostname", }, - "should add HOSTNAME=os.Hostname for host network namespace if sandbox.Hostname is not set": { + { + desc: "should add HOSTNAME=os.Hostname for host network namespace if sandbox.Hostname is not set", hostname: "", networkNs: runtime.NamespaceMode_NODE, expectedEnv: "HOSTNAME=real-hostname", }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { sandboxConfig.Hostname = test.hostname sandboxConfig.Linux.SecurityContext = &runtime.LinuxSandboxSecurityContext{ NamespaceOptions: &runtime.NamespaceOption{Network: test.networkNs}, @@ -1494,6 +1602,7 @@ func TestGenerateUserString(t *testing.T) { }, } for _, tc := range testcases { + tc := tc t.Run(tc.name, func(t *testing.T) { r, err := generateUserString(tc.u, tc.uid, tc.gid) if tc.expectedError { @@ -1537,12 +1646,14 @@ additional-group-for-root:x:22222:root os.WriteFile(filepath.Join(tempRootDir, "etc", "group"), []byte(etcGroup), 0644), ) - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string imageConfigUser string securityContext *runtime.LinuxContainerSecurityContext expected runtimespec.User }{ - "Only SecurityContext was set, SecurityContext defines User": { + { + desc: "Only SecurityContext was set, SecurityContext defines User", securityContext: &runtime.LinuxContainerSecurityContext{ RunAsUser: &runtime.Int64Value{Value: 1000}, RunAsGroup: &runtime.Int64Value{Value: 2000}, @@ -1550,12 +1661,14 @@ additional-group-for-root:x:22222:root }, expected: runtimespec.User{UID: 1000, GID: 2000, AdditionalGids: []uint32{2000, 3333, 11111}}, }, - "Only imageConfig.User was set, imageConfig.User defines User": { + { + desc: "Only imageConfig.User was set, imageConfig.User defines User", imageConfigUser: "1000", securityContext: nil, expected: runtimespec.User{UID: 1000, GID: 1000, AdditionalGids: []uint32{1000, 11111}}, }, - "Both SecurityContext and ImageConfig.User was set, SecurityContext defines User": { + { + desc: "Both SecurityContext and ImageConfig.User was set, SecurityContext defines User", imageConfigUser: "0", securityContext: &runtime.LinuxContainerSecurityContext{ RunAsUser: &runtime.Int64Value{Value: 1000}, @@ -1564,12 +1677,13 @@ additional-group-for-root:x:22222:root }, expected: runtimespec.User{UID: 1000, GID: 2000, AdditionalGids: []uint32{2000, 3333, 11111}}, }, - "No SecurityContext nor ImageConfig.User were set, runtime default defines User": { + { + desc: "No SecurityContext nor ImageConfig.User were set, runtime default defines User", expected: runtimespec.User{UID: 0, GID: 0, AdditionalGids: []uint32{0, 22222}}, }, } { - desc := desc - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { containerConfig, sandboxConfig, imageConfig, _ := getCreateContainerTestData() containerConfig.Linux.SecurityContext = test.securityContext imageConfig.User = test.imageConfigUser @@ -1599,32 +1713,37 @@ func TestNonRootUserAndDevices(t *testing.T) { testDevice := hostDevicesRaw[0] - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string uid, gid *runtime.Int64Value deviceOwnershipFromSecurityContext bool expectedDeviceUID uint32 expectedDeviceGID uint32 }{ - "expect non-root container's Devices Uid/Gid to be the same as the device Uid/Gid on the host when deviceOwnershipFromSecurityContext is disabled": { + { + desc: "expect non-root container's Devices Uid/Gid to be the same as the device Uid/Gid on the host when deviceOwnershipFromSecurityContext is disabled", uid: &runtime.Int64Value{Value: 1}, gid: &runtime.Int64Value{Value: 10}, expectedDeviceUID: *testDevice.UID, expectedDeviceGID: *testDevice.GID, }, - "expect root container's Devices Uid/Gid to be the same as the device Uid/Gid on the host when deviceOwnershipFromSecurityContext is disabled": { + { + desc: "expect root container's Devices Uid/Gid to be the same as the device Uid/Gid on the host when deviceOwnershipFromSecurityContext is disabled", uid: &runtime.Int64Value{Value: 0}, gid: &runtime.Int64Value{Value: 0}, expectedDeviceUID: *testDevice.UID, expectedDeviceGID: *testDevice.GID, }, - "expect non-root container's Devices Uid/Gid to be the same as RunAsUser/RunAsGroup when deviceOwnershipFromSecurityContext is enabled": { + { + desc: "expect non-root container's Devices Uid/Gid to be the same as RunAsUser/RunAsGroup when deviceOwnershipFromSecurityContext is enabled", uid: &runtime.Int64Value{Value: 1}, gid: &runtime.Int64Value{Value: 10}, deviceOwnershipFromSecurityContext: true, expectedDeviceUID: 1, expectedDeviceGID: 10, }, - "expect root container's Devices Uid/Gid to be the same as the device Uid/Gid on the host when deviceOwnershipFromSecurityContext is enabled": { + { + desc: "expect root container's Devices Uid/Gid to be the same as the device Uid/Gid on the host when deviceOwnershipFromSecurityContext is enabled", uid: &runtime.Int64Value{Value: 0}, gid: &runtime.Int64Value{Value: 0}, deviceOwnershipFromSecurityContext: true, @@ -1632,7 +1751,8 @@ func TestNonRootUserAndDevices(t *testing.T) { expectedDeviceGID: *testDevice.GID, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { c.config.DeviceOwnershipFromSecurityContext = test.deviceOwnershipFromSecurityContext containerConfig.Linux.SecurityContext.RunAsUser = test.uid containerConfig.Linux.SecurityContext.RunAsGroup = test.gid @@ -1660,42 +1780,48 @@ func TestPrivilegedDevices(t *testing.T) { testContainerName := "container-name" containerConfig, sandboxConfig, imageConfig, _ := getCreateContainerTestData() - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string privileged bool privilegedWithoutHostDevices bool privilegedWithoutHostDevicesAllDevicesAllowed bool expectHostDevices bool expectAllDevicesAllowed bool }{ - "expect no host devices when privileged is false": { + { + desc: "expect no host devices when privileged is false", privileged: false, privilegedWithoutHostDevices: false, privilegedWithoutHostDevicesAllDevicesAllowed: false, expectHostDevices: false, expectAllDevicesAllowed: false, }, - "expect no host devices when privileged is false and privilegedWithoutHostDevices is true": { + { + desc: "expect no host devices when privileged is false and privilegedWithoutHostDevices is true", privileged: false, privilegedWithoutHostDevices: true, privilegedWithoutHostDevicesAllDevicesAllowed: false, expectHostDevices: false, expectAllDevicesAllowed: false, }, - "expect host devices and all device allowlist when privileged is true": { + { + desc: "expect host devices and all device allowlist when privileged is true", privileged: true, privilegedWithoutHostDevices: false, privilegedWithoutHostDevicesAllDevicesAllowed: false, expectHostDevices: true, expectAllDevicesAllowed: true, }, - "expect no host devices when privileged is true and privilegedWithoutHostDevices is true": { + { + desc: "expect no host devices when privileged is true and privilegedWithoutHostDevices is true", privileged: true, privilegedWithoutHostDevices: true, privilegedWithoutHostDevicesAllDevicesAllowed: false, expectHostDevices: false, expectAllDevicesAllowed: false, }, - "expect host devices and all devices allowlist when privileged is true and privilegedWithoutHostDevices is true and privilegedWithoutHostDevicesAllDevicesAllowed is true": { + { + desc: "expect host devices and all devices allowlist when privileged is true and privilegedWithoutHostDevices is true and privilegedWithoutHostDevicesAllDevicesAllowed is true", privileged: true, privilegedWithoutHostDevices: true, privilegedWithoutHostDevicesAllDevicesAllowed: true, @@ -1703,7 +1829,8 @@ func TestPrivilegedDevices(t *testing.T) { expectAllDevicesAllowed: true, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { containerConfig.Linux.SecurityContext.Privileged = test.privileged sandboxConfig.Linux.SecurityContext.Privileged = test.privileged @@ -1898,6 +2025,7 @@ containerEdits: }, }, } { + test := test t.Run(test.description, func(t *testing.T) { spec, err := c.containerSpec(testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime) require.NoError(t, err) diff --git a/pkg/cri/server/container_create_test.go b/pkg/cri/server/container_create_test.go index 0bec679e3..df65106cd 100644 --- a/pkg/cri/server/container_create_test.go +++ b/pkg/cri/server/container_create_test.go @@ -81,18 +81,21 @@ func TestPodAnnotationPassthroughContainerSpec(t *testing.T) { testContainerName := "container-name" testPid := uint32(1234) - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string podAnnotations []string configChange func(*runtime.PodSandboxConfig) specCheck func(*testing.T, *runtimespec.Spec) }{ - "a passthrough annotation should be passed as an OCI annotation": { + { + desc: "a passthrough annotation should be passed as an OCI annotation", podAnnotations: []string{"c"}, specCheck: func(t *testing.T, spec *runtimespec.Spec) { assert.Equal(t, spec.Annotations["c"], "d") }, }, - "a non-passthrough annotation should not be passed as an OCI annotation": { + { + desc: "a non-passthrough annotation should not be passed as an OCI annotation", configChange: func(c *runtime.PodSandboxConfig) { c.Annotations["d"] = "e" }, @@ -103,7 +106,8 @@ func TestPodAnnotationPassthroughContainerSpec(t *testing.T) { assert.False(t, ok) }, }, - "passthrough annotations should support wildcard match": { + { + desc: "passthrough annotations should support wildcard match", configChange: func(c *runtime.PodSandboxConfig) { c.Annotations["t.f"] = "j" c.Annotations["z.g"] = "o" @@ -124,7 +128,8 @@ func TestPodAnnotationPassthroughContainerSpec(t *testing.T) { }, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { c := newTestCRIService() containerConfig, sandboxConfig, imageConfig, specCheck := getCreateContainerTestData() if test.configChange != nil { @@ -147,7 +152,8 @@ func TestPodAnnotationPassthroughContainerSpec(t *testing.T) { } func TestContainerSpecCommand(t *testing.T) { - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string criEntrypoint []string criArgs []string imageEntrypoint []string @@ -155,42 +161,49 @@ func TestContainerSpecCommand(t *testing.T) { expected []string expectErr bool }{ - "should use cri entrypoint if it's specified": { + { + desc: "should use cri entrypoint if it's specified", criEntrypoint: []string{"a", "b"}, imageEntrypoint: []string{"c", "d"}, imageArgs: []string{"e", "f"}, expected: []string{"a", "b"}, }, - "should use cri entrypoint if it's specified even if it's empty": { + { + desc: "should use cri entrypoint if it's specified even if it's empty", criEntrypoint: []string{}, criArgs: []string{"a", "b"}, imageEntrypoint: []string{"c", "d"}, imageArgs: []string{"e", "f"}, expected: []string{"a", "b"}, }, - "should use cri entrypoint and args if they are specified": { + { + desc: "should use cri entrypoint and args if they are specified", criEntrypoint: []string{"a", "b"}, criArgs: []string{"c", "d"}, imageEntrypoint: []string{"e", "f"}, imageArgs: []string{"g", "h"}, expected: []string{"a", "b", "c", "d"}, }, - "should use image entrypoint if cri entrypoint is not specified": { + { + desc: "should use image entrypoint if cri entrypoint is not specified", criArgs: []string{"a", "b"}, imageEntrypoint: []string{"c", "d"}, imageArgs: []string{"e", "f"}, expected: []string{"c", "d", "a", "b"}, }, - "should use image args if both cri entrypoint and args are not specified": { + { + desc: "should use image args if both cri entrypoint and args are not specified", imageEntrypoint: []string{"c", "d"}, imageArgs: []string{"e", "f"}, expected: []string{"c", "d", "e", "f"}, }, - "should return error if both entrypoint and args are empty": { + { + desc: "should return error if both entrypoint and args are empty", expectErr: true, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { config, _, imageConfig, _ := getCreateContainerTestData() config.Command = test.criEntrypoint config.Args = test.criArgs @@ -204,19 +217,21 @@ func TestContainerSpecCommand(t *testing.T) { return } assert.NoError(t, err) - assert.Equal(t, test.expected, spec.Process.Args, desc) + assert.Equal(t, test.expected, spec.Process.Args, test.desc) }) } } func TestVolumeMounts(t *testing.T) { testContainerRootDir := "test-container-root" - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string criMounts []*runtime.Mount imageVolumes map[string]struct{} expectedMountDest []string }{ - "should setup rw mount for image volumes": { + { + desc: "should setup rw mount for image volumes", imageVolumes: map[string]struct{}{ "/test-volume-1": {}, "/test-volume-2": {}, @@ -226,7 +241,8 @@ func TestVolumeMounts(t *testing.T) { "/test-volume-2", }, }, - "should skip image volumes if already mounted by CRI": { + { + desc: "should skip image volumes if already mounted by CRI", criMounts: []*runtime.Mount{ { ContainerPath: "/test-volume-1", @@ -241,7 +257,8 @@ func TestVolumeMounts(t *testing.T) { "/test-volume-2", }, }, - "should compare and return cleanpath": { + { + desc: "should compare and return cleanpath", criMounts: []*runtime.Mount{ { ContainerPath: "/test-volume-1", @@ -257,7 +274,8 @@ func TestVolumeMounts(t *testing.T) { }, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { config := &imagespec.ImageConfig{ Volumes: test.imageVolumes, } @@ -294,14 +312,16 @@ func TestContainerAnnotationPassthroughContainerSpec(t *testing.T) { testContainerName := "container-name" testPid := uint32(1234) - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string podAnnotations []string containerAnnotations []string podConfigChange func(*runtime.PodSandboxConfig) configChange func(*runtime.ContainerConfig) specCheck func(*testing.T, *runtimespec.Spec) }{ - "passthrough annotations from pod and container should be passed as an OCI annotation": { + { + desc: "passthrough annotations from pod and container should be passed as an OCI annotation", podConfigChange: func(p *runtime.PodSandboxConfig) { p.Annotations["pod.annotation.1"] = "1" p.Annotations["pod.annotation.2"] = "2" @@ -327,7 +347,8 @@ func TestContainerAnnotationPassthroughContainerSpec(t *testing.T) { assert.False(t, ok) }, }, - "passthrough annotations from pod and container should support wildcard": { + { + desc: "passthrough annotations from pod and container should support wildcard", podConfigChange: func(p *runtime.PodSandboxConfig) { p.Annotations["pod.annotation.1"] = "1" p.Annotations["pod.annotation.2"] = "2" @@ -349,7 +370,8 @@ func TestContainerAnnotationPassthroughContainerSpec(t *testing.T) { assert.Equal(t, "3", spec.Annotations["pod.annotation.3"]) }, }, - "annotations should not pass through if no passthrough annotations are configured": { + { + desc: "annotations should not pass through if no passthrough annotations are configured", podConfigChange: func(p *runtime.PodSandboxConfig) { p.Annotations["pod.annotation.1"] = "1" p.Annotations["pod.annotation.2"] = "2" @@ -378,7 +400,8 @@ func TestContainerAnnotationPassthroughContainerSpec(t *testing.T) { }, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { c := newTestCRIService() containerConfig, sandboxConfig, imageConfig, specCheck := getCreateContainerTestData() if test.configChange != nil { @@ -440,20 +463,24 @@ func TestRuntimeSnapshotter(t *testing.T) { Snapshotter: "devmapper", } - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string runtime config.Runtime expectSnapshotter string }{ - "should return default snapshotter when runtime.Snapshotter is not set": { + { + desc: "should return default snapshotter when runtime.Snapshotter is not set", runtime: defaultRuntime, expectSnapshotter: config.DefaultConfig().Snapshotter, }, - "should return overridden snapshotter when runtime.Snapshotter is set": { + { + desc: "should return overridden snapshotter when runtime.Snapshotter is set", runtime: fooRuntime, expectSnapshotter: "devmapper", }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { cri := newTestCRIService() cri.config = config.Config{ PluginConfig: config.DefaultConfig(), diff --git a/pkg/cri/server/container_create_windows_test.go b/pkg/cri/server/container_create_windows_test.go index 7f8578b8f..706c05253 100644 --- a/pkg/cri/server/container_create_windows_test.go +++ b/pkg/cri/server/container_create_windows_test.go @@ -213,33 +213,39 @@ func TestHostProcessRequirements(t *testing.T) { containerConfig, sandboxConfig, imageConfig, _ := getCreateContainerTestData() ociRuntime := config.Runtime{} c := newTestCRIService() - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string containerHostProcess bool sandboxHostProcess bool expectError bool }{ - "hostprocess container in non-hostprocess sandbox should fail": { + { + desc: "hostprocess container in non-hostprocess sandbox should fail", containerHostProcess: true, sandboxHostProcess: false, expectError: true, }, - "hostprocess container in hostprocess sandbox should be fine": { + { + desc: "hostprocess container in hostprocess sandbox should be fine", containerHostProcess: true, sandboxHostProcess: true, expectError: false, }, - "non-hostprocess container in hostprocess sandbox should fail": { + { + desc: "non-hostprocess container in hostprocess sandbox should fail", containerHostProcess: false, sandboxHostProcess: true, expectError: true, }, - "non-hostprocess container in non-hostprocess sandbox should be fine": { + { + desc: "non-hostprocess container in non-hostprocess sandbox should be fine", containerHostProcess: false, sandboxHostProcess: false, expectError: false, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { containerConfig.Windows.SecurityContext.HostProcess = test.containerHostProcess sandboxConfig.Windows.SecurityContext = &runtime.WindowsSandboxSecurityContext{ HostProcess: test.sandboxHostProcess, @@ -262,7 +268,8 @@ func TestEntrypointAndCmdForArgsEscaped(t *testing.T) { nsPath := "test-ns" c := newTestCRIService() - for name, test := range map[string]struct { + for _, test := range []struct { + name string imgEntrypoint []string imgCmd []string command []string @@ -272,7 +279,8 @@ func TestEntrypointAndCmdForArgsEscaped(t *testing.T) { ArgsEscaped bool }{ // override image entrypoint and cmd in shell form with container args and verify expected runtime spec - "TestShellFormImgEntrypointCmdWithCtrArgs": { + { + name: "TestShellFormImgEntrypointCmdWithCtrArgs", imgEntrypoint: []string{`"C:\My Folder\MyProcess.exe" -arg1 "test value"`}, imgCmd: []string{`cmd -args "hello world"`}, command: nil, @@ -282,7 +290,8 @@ func TestEntrypointAndCmdForArgsEscaped(t *testing.T) { ArgsEscaped: true, }, // check image entrypoint and cmd in shell form without overriding with container command and args and verify expected runtime spec - "TestShellFormImgEntrypointCmdWithoutCtrArgs": { + { + name: "TestShellFormImgEntrypointCmdWithoutCtrArgs", imgEntrypoint: []string{`"C:\My Folder\MyProcess.exe" -arg1 "test value"`}, imgCmd: []string{`cmd -args "hello world"`}, command: nil, @@ -292,7 +301,8 @@ func TestEntrypointAndCmdForArgsEscaped(t *testing.T) { ArgsEscaped: true, }, // override image entrypoint and cmd by container command and args in shell form and verify expected runtime spec - "TestShellFormImgEntrypointCmdWithCtrEntrypointAndArgs": { + { + name: "TestShellFormImgEntrypointCmdWithCtrEntrypointAndArgs", imgEntrypoint: []string{`"C:\My Folder\MyProcess.exe" -arg1 "test value"`}, imgCmd: []string{`cmd -args "hello world"`}, command: []string{`C:\My Folder\MyProcess.exe`, "-arg1", "additional test value"}, @@ -302,7 +312,8 @@ func TestEntrypointAndCmdForArgsEscaped(t *testing.T) { ArgsEscaped: true, }, // override image cmd by container args in exec form and verify expected runtime spec - "TestExecFormImgEntrypointCmdWithCtrArgs": { + { + name: "TestExecFormImgEntrypointCmdWithCtrArgs", imgEntrypoint: []string{`C:\My Folder\MyProcess.exe`, "-arg1", "test value"}, imgCmd: []string{"cmd", "-args", "hello world"}, command: nil, @@ -312,7 +323,8 @@ func TestEntrypointAndCmdForArgsEscaped(t *testing.T) { ArgsEscaped: false, }, // check image entrypoint and cmd in exec form without overriding with container command and args and verify expected runtime spec - "TestExecFormImgEntrypointCmdWithoutCtrArgs": { + { + name: "TestExecFormImgEntrypointCmdWithoutCtrArgs", imgEntrypoint: []string{`C:\My Folder\MyProcess.exe`, "-arg1", "test value"}, imgCmd: []string{"cmd", "-args", "hello world"}, command: nil, @@ -322,7 +334,8 @@ func TestEntrypointAndCmdForArgsEscaped(t *testing.T) { ArgsEscaped: false, }, } { - t.Run(name, func(t *testing.T) { + test := test + t.Run(test.name, func(t *testing.T) { imageConfig := &imagespec.ImageConfig{ Entrypoint: test.imgEntrypoint, Cmd: test.imgCmd, diff --git a/pkg/cri/server/container_list_test.go b/pkg/cri/server/container_list_test.go index 8a27cad97..8af7165b2 100644 --- a/pkg/cri/server/container_list_test.go +++ b/pkg/cri/server/container_list_test.go @@ -101,18 +101,22 @@ func TestFilterContainers(t *testing.T) { Labels: map[string]string{"c": "d"}, }, } - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string filter *runtime.ContainerFilter expect []*runtime.Container }{ - "no filter": { + { + desc: "no filter", expect: testContainers, }, - "id filter": { + { + desc: "id filter", filter: &runtime.ContainerFilter{Id: "2"}, expect: []*runtime.Container{testContainers[1]}, }, - "state filter": { + { + desc: "state filter", filter: &runtime.ContainerFilter{ State: &runtime.ContainerStateValue{ State: runtime.ContainerState_CONTAINER_EXITED, @@ -120,17 +124,20 @@ func TestFilterContainers(t *testing.T) { }, expect: []*runtime.Container{testContainers[1]}, }, - "label filter": { + { + desc: "label filter", filter: &runtime.ContainerFilter{ LabelSelector: map[string]string{"a": "b"}, }, expect: []*runtime.Container{testContainers[1]}, }, - "sandbox id filter": { + { + desc: "sandbox id filter", filter: &runtime.ContainerFilter{PodSandboxId: "s-2"}, expect: []*runtime.Container{testContainers[1], testContainers[2]}, }, - "mixed filter not matched": { + { + desc: "mixed filter not matched", filter: &runtime.ContainerFilter{ Id: "1", PodSandboxId: "s-2", @@ -138,7 +145,8 @@ func TestFilterContainers(t *testing.T) { }, expect: []*runtime.Container{}, }, - "mixed filter matched": { + { + desc: "mixed filter matched", filter: &runtime.ContainerFilter{ PodSandboxId: "s-2", State: &runtime.ContainerStateValue{ @@ -149,9 +157,10 @@ func TestFilterContainers(t *testing.T) { expect: []*runtime.Container{testContainers[2]}, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { filtered := c.filterCRIContainers(testContainers, test.filter) - assert.Equal(t, test.expect, filtered, desc) + assert.Equal(t, test.expect, filtered, test.desc) }) } } @@ -287,46 +296,54 @@ func TestListContainers(t *testing.T) { assert.NoError(t, c.containerStore.Add(container)) } - for testdesc, testdata := range map[string]struct { + for _, testdata := range []struct { + desc string filter *runtime.ContainerFilter expect []*runtime.Container }{ - "test without filter": { + { + desc: "test without filter", filter: &runtime.ContainerFilter{}, expect: expectedContainers, }, - "test filter by sandboxid": { + { + desc: "test filter by sandboxid", filter: &runtime.ContainerFilter{ PodSandboxId: "s-1abcdef1234", }, expect: expectedContainers[:3], }, - "test filter by truncated sandboxid": { + { + desc: "test filter by truncated sandboxid", filter: &runtime.ContainerFilter{ PodSandboxId: "s-1", }, expect: expectedContainers[:3], }, - "test filter by containerid": { + { + desc: "test filter by containerid", filter: &runtime.ContainerFilter{ Id: "c-1container", }, expect: expectedContainers[:1], }, - "test filter by truncated containerid": { + { + desc: "test filter by truncated containerid", filter: &runtime.ContainerFilter{ Id: "c-1", }, expect: expectedContainers[:1], }, - "test filter by containerid and sandboxid": { + { + desc: "test filter by containerid and sandboxid", filter: &runtime.ContainerFilter{ Id: "c-1container", PodSandboxId: "s-1abcdef1234", }, expect: expectedContainers[:1], }, - "test filter by truncated containerid and truncated sandboxid": { + { + desc: "test filter by truncated containerid and truncated sandboxid", filter: &runtime.ContainerFilter{ Id: "c-1", PodSandboxId: "s-1", @@ -334,7 +351,8 @@ func TestListContainers(t *testing.T) { expect: expectedContainers[:1], }, } { - t.Run(testdesc, func(t *testing.T) { + testdata := testdata + t.Run(testdata.desc, func(t *testing.T) { resp, err := c.ListContainers(context.Background(), &runtime.ListContainersRequest{Filter: testdata.filter}) assert.NoError(t, err) require.NotNil(t, resp) diff --git a/pkg/cri/server/container_remove_test.go b/pkg/cri/server/container_remove_test.go index e9b181d9b..6beda4616 100644 --- a/pkg/cri/server/container_remove_test.go +++ b/pkg/cri/server/container_remove_test.go @@ -29,25 +29,29 @@ import ( // state correctly. func TestSetContainerRemoving(t *testing.T) { testID := "test-id" - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string status containerstore.Status expectErr bool }{ - "should return error when container is in running state": { + { + desc: "should return error when container is in running state", status: containerstore.Status{ CreatedAt: time.Now().UnixNano(), StartedAt: time.Now().UnixNano(), }, expectErr: true, }, - "should return error when container is in starting state": { + { + desc: "should return error when container is in starting state", status: containerstore.Status{ CreatedAt: time.Now().UnixNano(), Starting: true, }, expectErr: true, }, - "should return error when container is in removing state": { + { + desc: "should return error when container is in removing state", status: containerstore.Status{ CreatedAt: time.Now().UnixNano(), StartedAt: time.Now().UnixNano(), @@ -56,7 +60,8 @@ func TestSetContainerRemoving(t *testing.T) { }, expectErr: true, }, - "should not return error when container is not running and removing": { + { + desc: "should not return error when container is not running and removing", status: containerstore.Status{ CreatedAt: time.Now().UnixNano(), StartedAt: time.Now().UnixNano(), @@ -65,7 +70,8 @@ func TestSetContainerRemoving(t *testing.T) { expectErr: false, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { container, err := containerstore.NewContainer( containerstore.Metadata{ID: testID}, containerstore.WithFakeStatus(test.status), diff --git a/pkg/cri/server/container_start_test.go b/pkg/cri/server/container_start_test.go index 4ab50fde0..127cfc98f 100644 --- a/pkg/cri/server/container_start_test.go +++ b/pkg/cri/server/container_start_test.go @@ -29,25 +29,29 @@ import ( // state correctly. func TestSetContainerStarting(t *testing.T) { testID := "test-id" - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string status containerstore.Status expectErr bool }{ - "should not return error when container is in created state": { + { + desc: "should not return error when container is in created state", status: containerstore.Status{ CreatedAt: time.Now().UnixNano(), }, expectErr: false, }, - "should return error when container is in running state": { + { + desc: "should return error when container is in running state", status: containerstore.Status{ CreatedAt: time.Now().UnixNano(), StartedAt: time.Now().UnixNano(), }, expectErr: true, }, - "should return error when container is in exited state": { + { + desc: "should return error when container is in exited state", status: containerstore.Status{ CreatedAt: time.Now().UnixNano(), StartedAt: time.Now().UnixNano(), @@ -55,7 +59,8 @@ func TestSetContainerStarting(t *testing.T) { }, expectErr: true, }, - "should return error when container is in unknown state": { + { + desc: "should return error when container is in unknown state", status: containerstore.Status{ CreatedAt: 0, StartedAt: 0, @@ -63,14 +68,16 @@ func TestSetContainerStarting(t *testing.T) { }, expectErr: true, }, - "should return error when container is in starting state": { + { + desc: "should return error when container is in starting state", status: containerstore.Status{ CreatedAt: time.Now().UnixNano(), Starting: true, }, expectErr: true, }, - "should return error when container is in removing state": { + { + desc: "should return error when container is in removing state", status: containerstore.Status{ CreatedAt: time.Now().UnixNano(), Removing: true, @@ -78,7 +85,8 @@ func TestSetContainerStarting(t *testing.T) { expectErr: true, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { container, err := containerstore.NewContainer( containerstore.Metadata{ID: testID}, containerstore.WithFakeStatus(test.status), diff --git a/pkg/cri/server/container_stats_list_linux_test.go b/pkg/cri/server/container_stats_list_linux_test.go index 01f0503f5..f061a412f 100644 --- a/pkg/cri/server/container_stats_list_linux_test.go +++ b/pkg/cri/server/container_stats_list_linux_test.go @@ -28,22 +28,26 @@ import ( ) func TestGetWorkingSet(t *testing.T) { - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string memory *v1.MemoryStat expected uint64 }{ - "nil memory usage": { + { + desc: "nil memory usage", memory: &v1.MemoryStat{}, expected: 0, }, - "memory usage higher than inactive_total_file": { + { + desc: "memory usage higher than inactive_total_file", memory: &v1.MemoryStat{ TotalInactiveFile: 1000, Usage: &v1.MemoryEntry{Usage: 2000}, }, expected: 1000, }, - "memory usage lower than inactive_total_file": { + { + desc: "memory usage lower than inactive_total_file", memory: &v1.MemoryStat{ TotalInactiveFile: 2000, Usage: &v1.MemoryEntry{Usage: 1000}, @@ -51,7 +55,8 @@ func TestGetWorkingSet(t *testing.T) { expected: 0, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { got := getWorkingSet(test.memory) assert.Equal(t, test.expected, got) }) @@ -59,22 +64,26 @@ func TestGetWorkingSet(t *testing.T) { } func TestGetWorkingSetV2(t *testing.T) { - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string memory *v2.MemoryStat expected uint64 }{ - "nil memory usage": { + { + desc: "nil memory usage", memory: &v2.MemoryStat{}, expected: 0, }, - "memory usage higher than inactive_total_file": { + { + desc: "memory usage higher than inactive_total_file", memory: &v2.MemoryStat{ InactiveFile: 1000, Usage: 2000, }, expected: 1000, }, - "memory usage lower than inactive_total_file": { + { + desc: "memory usage lower than inactive_total_file", memory: &v2.MemoryStat{ InactiveFile: 2000, Usage: 1000, @@ -82,7 +91,8 @@ func TestGetWorkingSetV2(t *testing.T) { expected: 0, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { got := getWorkingSetV2(test.memory) assert.Equal(t, test.expected, got) }) @@ -90,13 +100,15 @@ func TestGetWorkingSetV2(t *testing.T) { } func TestGetAvailableBytes(t *testing.T) { - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string memory *v1.MemoryStat workingSetBytes uint64 expected uint64 }{ - "no limit": { + { + desc: "no limit", memory: &v1.MemoryStat{ Usage: &v1.MemoryEntry{ Limit: math.MaxUint64, // no limit @@ -106,7 +118,8 @@ func TestGetAvailableBytes(t *testing.T) { workingSetBytes: 500, expected: 0, }, - "with limit": { + { + desc: "with limit", memory: &v1.MemoryStat{ Usage: &v1.MemoryEntry{ Limit: 5000, @@ -117,7 +130,8 @@ func TestGetAvailableBytes(t *testing.T) { expected: 5000 - 500, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { got := getAvailableBytes(test.memory, test.workingSetBytes) assert.Equal(t, test.expected, got) }) @@ -125,13 +139,15 @@ func TestGetAvailableBytes(t *testing.T) { } func TestGetAvailableBytesV2(t *testing.T) { - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string memory *v2.MemoryStat workingSetBytes uint64 expected uint64 }{ - "no limit": { + { + desc: "no limit", memory: &v2.MemoryStat{ UsageLimit: math.MaxUint64, // no limit Usage: 1000, @@ -139,7 +155,8 @@ func TestGetAvailableBytesV2(t *testing.T) { workingSetBytes: 500, expected: 0, }, - "with limit": { + { + desc: "with limit", memory: &v2.MemoryStat{ UsageLimit: 5000, Usage: 1000, @@ -148,7 +165,8 @@ func TestGetAvailableBytesV2(t *testing.T) { expected: 5000 - 500, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { got := getAvailableBytesV2(test.memory, test.workingSetBytes) assert.Equal(t, test.expected, got) }) @@ -159,11 +177,13 @@ func TestContainerMetricsMemory(t *testing.T) { c := newTestCRIService() timestamp := time.Now() - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string metrics interface{} expected *runtime.MemoryUsage }{ - "v1 metrics - no memory limit": { + { + desc: "v1 metrics - no memory limit", metrics: &v1.Metrics{ Memory: &v1.MemoryStat{ Usage: &v1.MemoryEntry{ @@ -186,7 +206,8 @@ func TestContainerMetricsMemory(t *testing.T) { MajorPageFaults: &runtime.UInt64Value{Value: 12}, }, }, - "v1 metrics - memory limit": { + { + desc: "v1 metrics - memory limit", metrics: &v1.Metrics{ Memory: &v1.MemoryStat{ Usage: &v1.MemoryEntry{ @@ -209,7 +230,8 @@ func TestContainerMetricsMemory(t *testing.T) { MajorPageFaults: &runtime.UInt64Value{Value: 12}, }, }, - "v2 metrics - memory limit": { + { + desc: "v2 metrics - memory limit", metrics: &v2.Metrics{ Memory: &v2.MemoryStat{ Usage: 1000, @@ -229,7 +251,8 @@ func TestContainerMetricsMemory(t *testing.T) { MajorPageFaults: &runtime.UInt64Value{Value: 12}, }, }, - "v2 metrics - no memory limit": { + { + desc: "v2 metrics - no memory limit", metrics: &v2.Metrics{ Memory: &v2.MemoryStat{ Usage: 1000, @@ -250,7 +273,8 @@ func TestContainerMetricsMemory(t *testing.T) { }, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { got, err := c.memoryContainerStats("ID", test.metrics, timestamp) assert.NoError(t, err) assert.Equal(t, test.expected, got) diff --git a/pkg/cri/server/container_stats_list_test.go b/pkg/cri/server/container_stats_list_test.go index 80dd851ec..70b34d991 100644 --- a/pkg/cri/server/container_stats_list_test.go +++ b/pkg/cri/server/container_stats_list_test.go @@ -30,20 +30,23 @@ func TestContainerMetricsCPUNanoCoreUsage(t *testing.T) { secondAfterTimeStamp := timestamp.Add(time.Second) ID := "ID" - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string firstCPUValue uint64 secondCPUValue uint64 expectedNanoCoreUsageFirst uint64 expectedNanoCoreUsageSecond uint64 }{ - "metrics": { + { + desc: "metrics", firstCPUValue: 50, secondCPUValue: 500, expectedNanoCoreUsageFirst: 0, expectedNanoCoreUsageSecond: 450, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { container, err := containerstore.NewContainer( containerstore.Metadata{ID: ID}, ) diff --git a/pkg/cri/server/container_status_test.go b/pkg/cri/server/container_status_test.go index 7df5277f1..9464d7ae8 100644 --- a/pkg/cri/server/container_status_test.go +++ b/pkg/cri/server/container_status_test.go @@ -85,7 +85,8 @@ func getContainerStatusTestData() (*containerstore.Metadata, *containerstore.Sta } func TestToCRIContainerStatus(t *testing.T) { - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string startedAt int64 finishedAt int64 exitCode int32 @@ -94,14 +95,17 @@ func TestToCRIContainerStatus(t *testing.T) { expectedState runtime.ContainerState expectedReason string }{ - "container created": { + { + desc: "container created", expectedState: runtime.ContainerState_CONTAINER_CREATED, }, - "container running": { + { + desc: "container running", startedAt: time.Now().UnixNano(), expectedState: runtime.ContainerState_CONTAINER_RUNNING, }, - "container exited with reason": { + { + desc: "container exited with reason", startedAt: time.Now().UnixNano(), finishedAt: time.Now().UnixNano(), exitCode: 1, @@ -110,7 +114,8 @@ func TestToCRIContainerStatus(t *testing.T) { expectedState: runtime.ContainerState_CONTAINER_EXITED, expectedReason: "test-reason", }, - "container exited with exit code 0 without reason": { + { + desc: "container exited with exit code 0 without reason", startedAt: time.Now().UnixNano(), finishedAt: time.Now().UnixNano(), exitCode: 0, @@ -118,7 +123,8 @@ func TestToCRIContainerStatus(t *testing.T) { expectedState: runtime.ContainerState_CONTAINER_EXITED, expectedReason: completeExitReason, }, - "container exited with non-zero exit code without reason": { + { + desc: "container exited with non-zero exit code without reason", startedAt: time.Now().UnixNano(), finishedAt: time.Now().UnixNano(), exitCode: 1, @@ -127,7 +133,8 @@ func TestToCRIContainerStatus(t *testing.T) { expectedReason: errorExitReason, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { metadata, status, _, expected := getContainerStatusTestData() // Update status with test case. @@ -151,7 +158,7 @@ func TestToCRIContainerStatus(t *testing.T) { containerStatus := toCRIContainerStatus(container, expected.Image, expected.ImageRef) - assert.Equal(t, expected, containerStatus, desc) + assert.Equal(t, expected, containerStatus, test.desc) }) } } @@ -173,7 +180,8 @@ func TestToCRIContainerInfo(t *testing.T) { } func TestContainerStatus(t *testing.T) { - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string exist bool imageExist bool startedAt int64 @@ -182,18 +190,21 @@ func TestContainerStatus(t *testing.T) { expectedState runtime.ContainerState expectErr bool }{ - "container created": { + { + desc: "container created", exist: true, imageExist: true, expectedState: runtime.ContainerState_CONTAINER_CREATED, }, - "container running": { + { + desc: "container running", exist: true, imageExist: true, startedAt: time.Now().UnixNano(), expectedState: runtime.ContainerState_CONTAINER_RUNNING, }, - "container exited": { + { + desc: "container exited", exist: true, imageExist: true, startedAt: time.Now().UnixNano(), @@ -201,18 +212,21 @@ func TestContainerStatus(t *testing.T) { reason: "test-reason", expectedState: runtime.ContainerState_CONTAINER_EXITED, }, - "container not exist": { + { + desc: "container not exist", exist: false, imageExist: true, expectErr: true, }, - "image not exist": { + { + desc: "image not exist", exist: false, imageExist: false, expectErr: true, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { c := newTestCRIService() metadata, status, image, expected := getContainerStatusTestData() // Update status with test case. diff --git a/pkg/cri/server/container_stop_test.go b/pkg/cri/server/container_stop_test.go index 57ce39956..e3ab4829b 100644 --- a/pkg/cri/server/container_stop_test.go +++ b/pkg/cri/server/container_stop_test.go @@ -28,13 +28,15 @@ import ( func TestWaitContainerStop(t *testing.T) { id := "test-id" - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string status *containerstore.Status cancel bool timeout time.Duration expectErr bool }{ - "should return error if timeout exceeds": { + { + desc: "should return error if timeout exceeds", status: &containerstore.Status{ CreatedAt: time.Now().UnixNano(), StartedAt: time.Now().UnixNano(), @@ -42,7 +44,8 @@ func TestWaitContainerStop(t *testing.T) { timeout: 200 * time.Millisecond, expectErr: true, }, - "should return error if context is cancelled": { + { + desc: "should return error if context is cancelled", status: &containerstore.Status{ CreatedAt: time.Now().UnixNano(), StartedAt: time.Now().UnixNano(), @@ -51,7 +54,8 @@ func TestWaitContainerStop(t *testing.T) { cancel: true, expectErr: true, }, - "should not return error if container is stopped before timeout": { + { + desc: "should not return error if container is stopped before timeout", status: &containerstore.Status{ CreatedAt: time.Now().UnixNano(), StartedAt: time.Now().UnixNano(), @@ -61,7 +65,8 @@ func TestWaitContainerStop(t *testing.T) { expectErr: false, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { c := newTestCRIService() container, err := containerstore.NewContainer( containerstore.Metadata{ID: id}, @@ -81,7 +86,7 @@ func TestWaitContainerStop(t *testing.T) { ctx = timeoutCtx } err = c.waitContainerStop(ctx, container) - assert.Equal(t, test.expectErr, err != nil, desc) + assert.Equal(t, test.expectErr, err != nil, test.desc) }) } } diff --git a/pkg/cri/server/container_update_resources_linux_test.go b/pkg/cri/server/container_update_resources_linux_test.go index b303948c1..59f1720f4 100644 --- a/pkg/cri/server/container_update_resources_linux_test.go +++ b/pkg/cri/server/container_update_resources_linux_test.go @@ -38,13 +38,15 @@ func TestUpdateOCILinuxResource(t *testing.T) { } return nil } - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string spec *runtimespec.Spec request *runtime.UpdateContainerResourcesRequest expected *runtimespec.Spec expectErr bool }{ - "should be able to update each resource": { + { + desc: "should be able to update each resource", spec: &runtimespec.Spec{ Process: &runtimespec.Process{OOMScoreAdj: oomscoreadj}, Linux: &runtimespec.Linux{ @@ -93,7 +95,8 @@ func TestUpdateOCILinuxResource(t *testing.T) { }, }, }, - "should skip empty fields": { + { + desc: "should skip empty fields", spec: &runtimespec.Spec{ Process: &runtimespec.Process{OOMScoreAdj: oomscoreadj}, Linux: &runtimespec.Linux{ @@ -139,7 +142,8 @@ func TestUpdateOCILinuxResource(t *testing.T) { }, }, }, - "should be able to fill empty fields": { + { + desc: "should be able to fill empty fields", spec: &runtimespec.Spec{ Process: &runtimespec.Process{OOMScoreAdj: oomscoreadj}, Linux: &runtimespec.Linux{ @@ -180,7 +184,8 @@ func TestUpdateOCILinuxResource(t *testing.T) { }, }, }, - "should be able to patch the unified map": { + { + desc: "should be able to patch the unified map", spec: &runtimespec.Spec{ Process: &runtimespec.Process{OOMScoreAdj: oomscoreadj}, Linux: &runtimespec.Linux{ @@ -230,7 +235,8 @@ func TestUpdateOCILinuxResource(t *testing.T) { }, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { config := criconfig.Config{ PluginConfig: criconfig.PluginConfig{ TolerateMissingHugetlbController: true, diff --git a/pkg/cri/server/helpers_linux_test.go b/pkg/cri/server/helpers_linux_test.go index 9c2c5e417..1249d3ad7 100644 --- a/pkg/cri/server/helpers_linux_test.go +++ b/pkg/cri/server/helpers_linux_test.go @@ -29,32 +29,38 @@ import ( func TestGetCgroupsPath(t *testing.T) { testID := "test-id" - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string cgroupsParent string expected string }{ - "should support regular cgroup path": { + { + desc: "should support regular cgroup path", cgroupsParent: "/a/b", expected: "/a/b/test-id", }, - "should support systemd cgroup path": { + { + desc: "should support systemd cgroup path", cgroupsParent: "/a.slice/b.slice", - expected: "b.slice:cri-containerd:test-id", - }, - "should support tailing slash for regular cgroup path": { + expected: "b.slice:cri-containerd:test-id"}, + { + desc: "should support tailing slash for regular cgroup path", cgroupsParent: "/a/b/", expected: "/a/b/test-id", }, - "should support tailing slash for systemd cgroup path": { + { + desc: "should support tailing slash for systemd cgroup path", cgroupsParent: "/a.slice/b.slice/", expected: "b.slice:cri-containerd:test-id", }, - "should treat root cgroup as regular cgroup path": { + { + desc: "should treat root cgroup as regular cgroup path", cgroupsParent: "/", expected: "/test-id", }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { got := getCgroupsPath(test.cgroupsParent, testID) assert.Equal(t, test.expected, got) }) diff --git a/pkg/cri/server/helpers_selinux_linux_test.go b/pkg/cri/server/helpers_selinux_linux_test.go index 6bccee21a..b81219184 100644 --- a/pkg/cri/server/helpers_selinux_linux_test.go +++ b/pkg/cri/server/helpers_selinux_linux_test.go @@ -29,18 +29,21 @@ func TestInitSelinuxOpts(t *testing.T) { t.Skip("selinux is not enabled") } - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string selinuxOpt *runtime.SELinuxOption processLabel string mountLabel string expectErr bool }{ - "Should return empty strings for processLabel and mountLabel when selinuxOpt is nil": { + { + desc: "Should return empty strings for processLabel and mountLabel when selinuxOpt is nil", selinuxOpt: nil, processLabel: ".*:c[0-9]{1,3},c[0-9]{1,3}", mountLabel: ".*:c[0-9]{1,3},c[0-9]{1,3}", }, - "Should overlay fields on processLabel when selinuxOpt has been initialized partially": { + { + desc: "Should overlay fields on processLabel when selinuxOpt has been initialized partially", selinuxOpt: &runtime.SELinuxOption{ User: "", Role: "user_r", @@ -50,7 +53,8 @@ func TestInitSelinuxOpts(t *testing.T) { processLabel: "system_u:user_r:(container_file_t|svirt_lxc_net_t):s0:c1,c2", mountLabel: "system_u:object_r:(container_file_t|svirt_sandbox_file_t):s0:c1,c2", }, - "Should be resolved correctly when selinuxOpt has been initialized completely": { + { + desc: "Should be resolved correctly when selinuxOpt has been initialized completely", selinuxOpt: &runtime.SELinuxOption{ User: "user_u", Role: "user_r", @@ -60,7 +64,8 @@ func TestInitSelinuxOpts(t *testing.T) { processLabel: "user_u:user_r:user_t:s0:c1,c2", mountLabel: "user_u:object_r:(container_file_t|svirt_sandbox_file_t):s0:c1,c2", }, - "Should be resolved correctly when selinuxOpt has been initialized with level=''": { + { + desc: "Should be resolved correctly when selinuxOpt has been initialized with level=''", selinuxOpt: &runtime.SELinuxOption{ User: "user_u", Role: "user_r", @@ -70,7 +75,8 @@ func TestInitSelinuxOpts(t *testing.T) { processLabel: "user_u:user_r:user_t:s0:c[0-9]{1,3},c[0-9]{1,3}", mountLabel: "user_u:object_r:(container_file_t|svirt_sandbox_file_t):s0", }, - "Should return error when the format of 'level' is not correct": { + { + desc: "Should return error when the format of 'level' is not correct", selinuxOpt: &runtime.SELinuxOption{ User: "user_u", Role: "user_r", @@ -80,7 +86,8 @@ func TestInitSelinuxOpts(t *testing.T) { expectErr: true, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { processLabel, mountLabel, err := initLabelsFromOpt(test.selinuxOpt) if test.expectErr { assert.Error(t, err) @@ -93,59 +100,75 @@ func TestInitSelinuxOpts(t *testing.T) { } func TestCheckSelinuxLevel(t *testing.T) { - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string level string expectNoMatch bool }{ - "s0": { + { + desc: "s0", level: "s0", }, - "s0-s0": { + { + desc: "s0-s0", level: "s0-s0", }, - "s0:c0": { + { + desc: "s0:c0", level: "s0:c0", }, - "s0:c0.c3": { + { + desc: "s0:c0.c3", level: "s0:c0.c3", }, - "s0:c0,c3": { + { + desc: "s0:c0,c3", level: "s0:c0,c3", }, - "s0-s0:c0,c3": { + { + desc: "s0-s0:c0,c3", level: "s0-s0:c0,c3", }, - "s0-s0:c0,c3.c6": { + { + desc: "s0-s0:c0,c3.c6", level: "s0-s0:c0,c3.c6", }, - "s0-s0:c0,c3.c6,c8.c10": { + { + desc: "s0-s0:c0,c3.c6,c8.c10", level: "s0-s0:c0,c3.c6,c8.c10", }, - "s0-s0:c0,c3.c6,c8,c10": { + { + desc: "s0-s0:c0,c3.c6,c8,c10", level: "s0-s0:c0,c3.c6", }, - "s0,c0,c3": { + { + desc: "s0,c0,c3", level: "s0,c0,c3", expectNoMatch: true, }, - "s0:c0.c3.c6": { + { + desc: "s0:c0.c3.c6", level: "s0:c0.c3.c6", expectNoMatch: true, }, - "s0-s0,c0,c3": { + { + desc: "s0-s0,c0,c3", level: "s0-s0,c0,c3", expectNoMatch: true, }, - "s0-s0:c0.c3.c6": { + { + desc: "s0-s0:c0.c3.c6", level: "s0-s0:c0.c3.c6", expectNoMatch: true, }, - "s0-s0:c0,c3.c6.c8": { + { + desc: "s0-s0:c0,c3.c6.c8", level: "s0-s0:c0,c3.c6.c8", expectNoMatch: true, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { err := checkSelinuxLevel(test.level) if test.expectNoMatch { assert.Error(t, err) diff --git a/pkg/cri/server/helpers_test.go b/pkg/cri/server/helpers_test.go index 8e609d28f..b47152b83 100644 --- a/pkg/cri/server/helpers_test.go +++ b/pkg/cri/server/helpers_test.go @@ -48,36 +48,44 @@ import ( // TestGetUserFromImage tests the logic of getting image uid or user name of image user. func TestGetUserFromImage(t *testing.T) { newI64 := func(i int64) *int64 { return &i } - for c, test := range map[string]struct { + for _, test := range []struct { + desc string user string uid *int64 name string }{ - "no gid": { + { + desc: "no gid", user: "0", uid: newI64(0), }, - "uid/gid": { + { + desc: "uid/gid", user: "0:1", uid: newI64(0), }, - "empty user": { + { + desc: "empty user", user: "", }, - "multiple separators": { + { + desc: "multiple separators", user: "1:2:3", uid: newI64(1), }, - "root username": { + { + desc: "root username", user: "root:root", name: "root", }, - "username": { + { + desc: "username", user: "test:test", name: "test", }, } { - t.Run(c, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { actualUID, actualName := getUserFromImage(test.user) assert.Equal(t, test.uid, actualUID) assert.Equal(t, test.name, actualName) @@ -87,35 +95,41 @@ func TestGetUserFromImage(t *testing.T) { func TestGetRepoDigestAndTag(t *testing.T) { digest := imagedigest.Digest("sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59582") - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string ref string schema1 bool expectedRepoDigest string expectedRepoTag string }{ - "repo tag should be empty if original ref has no tag": { + { + desc: "repo tag should be empty if original ref has no tag", ref: "gcr.io/library/busybox@" + digest.String(), expectedRepoDigest: "gcr.io/library/busybox@" + digest.String(), }, - "repo tag should not be empty if original ref has tag": { + { + desc: "repo tag should not be empty if original ref has tag", ref: "gcr.io/library/busybox:latest", expectedRepoDigest: "gcr.io/library/busybox@" + digest.String(), expectedRepoTag: "gcr.io/library/busybox:latest", }, - "repo digest should be empty if original ref is schema1 and has no digest": { + { + desc: "repo digest should be empty if original ref is schema1 and has no digest", ref: "gcr.io/library/busybox:latest", schema1: true, expectedRepoDigest: "", expectedRepoTag: "gcr.io/library/busybox:latest", }, - "repo digest should not be empty if original ref is schema1 but has digest": { + { + desc: "repo digest should not be empty if original ref is schema1 but has digest", ref: "gcr.io/library/busybox@sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59594", schema1: true, expectedRepoDigest: "gcr.io/library/busybox@sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59594", expectedRepoTag: "", }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { named, err := docker.ParseDockerRef(test.ref) assert.NoError(t, err) repoDigest, repoTag := getRepoDigestAndTag(named, digest, test.schema1) @@ -242,19 +256,22 @@ func TestGenerateRuntimeOptions(t *testing.T) { require.NoError(t, err) require.Len(t, nonNilOptsConfig.Runtimes, 3) - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string r criconfig.Runtime c criconfig.Config expectedOptions interface{} }{ - "when options is nil, should return nil option for io.containerd.runc.v2": { + { + desc: "when options is nil, should return nil option for io.containerd.runc.v2", r: nilOptsConfig.Runtimes["runcv2"], c: nilOptsConfig, expectedOptions: nil, }, - "when options is not nil, should be able to decode for io.containerd.runc.v2": { - r: nonNilOptsConfig.Runtimes["runcv2"], - c: nonNilOptsConfig, + { + desc: "when options is not nil, should be able to decode for io.containerd.runc.v2", + r: nonNilOptsConfig.Runtimes["runcv2"], + c: nonNilOptsConfig, expectedOptions: &runcoptions.Options{ BinaryName: "runc", Root: "/runcv2", @@ -262,7 +279,8 @@ func TestGenerateRuntimeOptions(t *testing.T) { }, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { opts, err := generateRuntimeOptions(test.r, test.c) assert.NoError(t, err) assert.Equal(t, test.expectedOptions, opts) @@ -271,18 +289,21 @@ func TestGenerateRuntimeOptions(t *testing.T) { } func TestEnvDeduplication(t *testing.T) { - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string existing []string kv [][2]string expected []string }{ - "single env": { + { + desc: "single env", kv: [][2]string{ {"a", "b"}, }, expected: []string{"a=b"}, }, - "multiple envs": { + { + desc: "multiple envs", kv: [][2]string{ {"a", "b"}, {"c", "d"}, @@ -294,7 +315,8 @@ func TestEnvDeduplication(t *testing.T) { "e=f", }, }, - "env override": { + { + desc: "env override", kv: [][2]string{ {"k1", "v1"}, {"k2", "v2"}, @@ -310,7 +332,8 @@ func TestEnvDeduplication(t *testing.T) { "k4=v6", }, }, - "existing env": { + { + desc: "existing env", existing: []string{ "k1=v1", "k2=v2", @@ -329,7 +352,8 @@ func TestEnvDeduplication(t *testing.T) { }, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { var spec runtimespec.Spec if len(test.existing) > 0 { spec.Process = &runtimespec.Process{ @@ -345,17 +369,20 @@ func TestEnvDeduplication(t *testing.T) { } func TestPassThroughAnnotationsFilter(t *testing.T) { - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string podAnnotations map[string]string runtimePodAnnotations []string passthroughAnnotations map[string]string }{ - "should support direct match": { + { + desc: "should support direct match", podAnnotations: map[string]string{"c": "d", "d": "e"}, runtimePodAnnotations: []string{"c"}, passthroughAnnotations: map[string]string{"c": "d"}, }, - "should support wildcard match": { + { + desc: "should support wildcard match", podAnnotations: map[string]string{ "t.f": "j", "z.g": "o", @@ -370,7 +397,8 @@ func TestPassThroughAnnotationsFilter(t *testing.T) { "y.ca": "b", }, }, - "should support wildcard match all": { + { + desc: "should support wildcard match all", podAnnotations: map[string]string{ "t.f": "j", "z.g": "o", @@ -387,7 +415,8 @@ func TestPassThroughAnnotationsFilter(t *testing.T) { "y": "b", }, }, - "should support match including path separator": { + { + desc: "should support match including path separator", podAnnotations: map[string]string{ "matchend.com/end": "1", "matchend.com/end1": "2", @@ -446,7 +475,8 @@ func TestPassThroughAnnotationsFilter(t *testing.T) { }, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { passthroughAnnotations := getPassthroughAnnotations(test.podAnnotations, test.runtimePodAnnotations) assert.Equal(t, test.passthroughAnnotations, passthroughAnnotations) }) @@ -534,28 +564,34 @@ func TestValidateTargetContainer(t *testing.T) { err = addContainer(c, testOtherContainerID, testOtherContainerSandboxID, testOtherContainerPID, createdAt, startedAt, 0) require.NoError(t, err, "error creating test container in other pod") - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string targetContainerID string expectError bool }{ - "target container in pod": { + { + desc: "target container in pod", targetContainerID: testTargetContainerID, expectError: false, }, - "target stopped container in pod": { + { + desc: "target stopped container in pod", targetContainerID: testStoppedContainerID, expectError: true, }, - "target container does not exist": { + { + desc: "target container does not exist", targetContainerID: "no-container-with-this-id", expectError: true, }, - "target container in other pod": { + { + desc: "target container in other pod", targetContainerID: testOtherContainerID, expectError: true, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { targetContainer, err := c.validateTargetContainer(testSandboxID, test.targetContainerID) if test.expectError { require.Error(t, err, "target should have been invalid but no error") @@ -617,6 +653,7 @@ func TestHostNetwork(t *testing.T) { if goruntime.GOOS != "linux" { t.Skip() } + tt := tt t.Run(tt.name, func(t *testing.T) { if hostNetwork(tt.c) != tt.expected { t.Errorf("failed hostNetwork got %t expected %t", hostNetwork(tt.c), tt.expected) diff --git a/pkg/cri/server/image_pull_test.go b/pkg/cri/server/image_pull_test.go index 428fb1cd4..a555ab5b6 100644 --- a/pkg/cri/server/image_pull_test.go +++ b/pkg/cri/server/image_pull_test.go @@ -37,23 +37,29 @@ func TestParseAuth(t *testing.T) { base64.StdEncoding.Encode(testAuth, []byte(testUser+":"+testPasswd)) invalidAuth := make([]byte, testAuthLen) base64.StdEncoding.Encode(invalidAuth, []byte(testUser+"@"+testPasswd)) - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string auth *runtime.AuthConfig host string expectedUser string expectedSecret string expectErr bool }{ - "should not return error if auth config is nil": {}, - "should not return error if empty auth is provided for access to anonymous registry": { + { + desc: "should not return error if auth config is nil", + }, + { + desc: "should not return error if empty auth is provided for access to anonymous registry", auth: &runtime.AuthConfig{}, expectErr: false, }, - "should support identity token": { + { + desc: "should support identity token", auth: &runtime.AuthConfig{IdentityToken: "abcd"}, expectedSecret: "abcd", }, - "should support username and password": { + { + desc: "should support username and password", auth: &runtime.AuthConfig{ Username: testUser, Password: testPasswd, @@ -61,16 +67,19 @@ func TestParseAuth(t *testing.T) { expectedUser: testUser, expectedSecret: testPasswd, }, - "should support auth": { + { + desc: "should support auth", auth: &runtime.AuthConfig{Auth: string(testAuth)}, expectedUser: testUser, expectedSecret: testPasswd, }, - "should return error for invalid auth": { + { + desc: "should return error for invalid auth", auth: &runtime.AuthConfig{Auth: string(invalidAuth)}, expectErr: true, }, - "should return empty auth if server address doesn't match": { + { + desc: "should return empty auth if server address doesn't match", auth: &runtime.AuthConfig{ Username: testUser, Password: testPasswd, @@ -80,7 +89,8 @@ func TestParseAuth(t *testing.T) { expectedUser: "", expectedSecret: "", }, - "should return auth if server address matches": { + { + desc: "should return auth if server address matches", auth: &runtime.AuthConfig{ Username: testUser, Password: testPasswd, @@ -90,7 +100,8 @@ func TestParseAuth(t *testing.T) { expectedUser: testUser, expectedSecret: testPasswd, }, - "should return auth if server address is not specified": { + { + desc: "should return auth if server address is not specified", auth: &runtime.AuthConfig{ Username: testUser, Password: testPasswd, @@ -100,7 +111,8 @@ func TestParseAuth(t *testing.T) { expectedSecret: testPasswd, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { u, s, err := ParseAuth(test.auth, test.host) assert.Equal(t, test.expectErr, err != nil) assert.Equal(t, test.expectedUser, u) @@ -110,12 +122,14 @@ func TestParseAuth(t *testing.T) { } func TestRegistryEndpoints(t *testing.T) { - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string mirrors map[string]criconfig.Mirror host string expected []string }{ - "no mirror configured": { + { + desc: "no mirror configured", mirrors: map[string]criconfig.Mirror{ "registry-1.io": { Endpoints: []string{ @@ -129,7 +143,8 @@ func TestRegistryEndpoints(t *testing.T) { "https://registry-3.io", }, }, - "mirror configured": { + { + desc: "mirror configured", mirrors: map[string]criconfig.Mirror{ "registry-3.io": { Endpoints: []string{ @@ -145,7 +160,8 @@ func TestRegistryEndpoints(t *testing.T) { "https://registry-3.io", }, }, - "wildcard mirror configured": { + { + desc: "wildcard mirror configured", mirrors: map[string]criconfig.Mirror{ "*": { Endpoints: []string{ @@ -161,7 +177,8 @@ func TestRegistryEndpoints(t *testing.T) { "https://registry-3.io", }, }, - "host should take precedence if both host and wildcard mirrors are configured": { + { + desc: "host should take precedence if both host and wildcard mirrors are configured", mirrors: map[string]criconfig.Mirror{ "*": { Endpoints: []string{ @@ -180,7 +197,8 @@ func TestRegistryEndpoints(t *testing.T) { "https://registry-3.io", }, }, - "default endpoint in list with http": { + { + desc: "default endpoint in list with http", mirrors: map[string]criconfig.Mirror{ "registry-3.io": { Endpoints: []string{ @@ -197,7 +215,8 @@ func TestRegistryEndpoints(t *testing.T) { "http://registry-3.io", }, }, - "default endpoint in list with https": { + { + desc: "default endpoint in list with https", mirrors: map[string]criconfig.Mirror{ "registry-3.io": { Endpoints: []string{ @@ -214,7 +233,8 @@ func TestRegistryEndpoints(t *testing.T) { "https://registry-3.io", }, }, - "default endpoint in list with path": { + { + desc: "default endpoint in list with path", mirrors: map[string]criconfig.Mirror{ "registry-3.io": { Endpoints: []string{ @@ -231,7 +251,8 @@ func TestRegistryEndpoints(t *testing.T) { "https://registry-3.io/path", }, }, - "miss scheme endpoint in list with path": { + { + desc: "miss scheme endpoint in list with path", mirrors: map[string]criconfig.Mirror{ "registry-3.io": { Endpoints: []string{ @@ -249,7 +270,8 @@ func TestRegistryEndpoints(t *testing.T) { }, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { c := newTestCRIService() c.config.Registry.Mirrors = test.mirrors got, err := c.registryEndpoints(test.host) @@ -260,52 +282,64 @@ func TestRegistryEndpoints(t *testing.T) { } func TestDefaultScheme(t *testing.T) { - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string host string expected string }{ - "should use http by default for localhost": { + { + desc: "should use http by default for localhost", host: "localhost", expected: "http", }, - "should use http by default for localhost with port": { + { + desc: "should use http by default for localhost with port", host: "localhost:8080", expected: "http", }, - "should use http by default for 127.0.0.1": { + { + desc: "should use http by default for 127.0.0.1", host: "127.0.0.1", expected: "http", }, - "should use http by default for 127.0.0.1 with port": { + { + desc: "should use http by default for 127.0.0.1 with port", host: "127.0.0.1:8080", expected: "http", }, - "should use http by default for ::1": { + { + desc: "should use http by default for ::1", host: "::1", expected: "http", }, - "should use http by default for ::1 with port": { + { + desc: "should use http by default for ::1 with port", host: "[::1]:8080", expected: "http", }, - "should use https by default for remote host": { + { + desc: "should use https by default for remote host", host: "remote", expected: "https", }, - "should use https by default for remote host with port": { + { + desc: "should use https by default for remote host with port", host: "remote:8080", expected: "https", }, - "should use https by default for remote ip": { + { + desc: "should use https by default for remote ip", host: "8.8.8.8", expected: "https", }, - "should use https by default for remote ip with port": { + { + desc: "should use https by default for remote ip with port", host: "8.8.8.8:8080", expected: "https", }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { got := defaultScheme(test.host) assert.Equal(t, test.expected, got) }) @@ -313,20 +347,24 @@ func TestDefaultScheme(t *testing.T) { } func TestEncryptedImagePullOpts(t *testing.T) { - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string keyModel string expectedOpts int }{ - "node key model should return one unpack opt": { + { + desc: "node key model should return one unpack opt", keyModel: criconfig.KeyModelNode, expectedOpts: 1, }, - "no key model selected should default to node key model": { + { + desc: "no key model selected should default to node key model", keyModel: "", expectedOpts: 0, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { c := newTestCRIService() c.config.ImageDecryption.KeyModel = test.keyModel got := len(c.encryptedImagesPullOpts()) @@ -382,6 +420,7 @@ func TestSnapshotterFromPodSandboxConfig(t *testing.T) { } for _, tt := range tests { + tt := tt t.Run(tt.desc, func(t *testing.T) { cri := newTestCRIService() cri.config.ContainerdConfig.Snapshotter = defaultSnashotter diff --git a/pkg/cri/server/sandbox_list_test.go b/pkg/cri/server/sandbox_list_test.go index 5c46bc7e2..1d38c98d4 100644 --- a/pkg/cri/server/sandbox_list_test.go +++ b/pkg/cri/server/sandbox_list_test.go @@ -53,31 +53,36 @@ func TestToCRISandbox(t *testing.T) { Annotations: config.GetAnnotations(), RuntimeHandler: "test-runtime-handler", } - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string state sandboxstore.State expectedState runtime.PodSandboxState }{ - "sandbox state ready": { + { + desc: "sandbox state ready", state: sandboxstore.StateReady, expectedState: runtime.PodSandboxState_SANDBOX_READY, }, - "sandbox state not ready": { + { + desc: "sandbox state not ready", state: sandboxstore.StateNotReady, expectedState: runtime.PodSandboxState_SANDBOX_NOTREADY, }, - "sandbox state unknown": { + { + desc: "sandbox state unknown", state: sandboxstore.StateUnknown, expectedState: runtime.PodSandboxState_SANDBOX_NOTREADY, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { status := sandboxstore.Status{ CreatedAt: createdAt, State: test.state, } expect.State = test.expectedState s := toCRISandbox(meta, status) - assert.Equal(t, expect, s, desc) + assert.Equal(t, expect, s, test.desc) }) } } @@ -157,22 +162,27 @@ func TestFilterSandboxes(t *testing.T) { assert.NoError(t, c.sandboxStore.Add(sb)) } - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string filter *runtime.PodSandboxFilter expect []*runtime.PodSandbox }{ - "no filter": { + { + desc: "no filter", expect: testSandboxes, }, - "id filter": { + { + desc: "id filter", filter: &runtime.PodSandboxFilter{Id: "2abcdef"}, expect: []*runtime.PodSandbox{testSandboxes[1]}, }, - "truncid filter": { + { + desc: "truncid filter", filter: &runtime.PodSandboxFilter{Id: "2"}, expect: []*runtime.PodSandbox{testSandboxes[1]}, }, - "state filter": { + { + desc: "state filter", filter: &runtime.PodSandboxFilter{ State: &runtime.PodSandboxStateValue{ State: runtime.PodSandboxState_SANDBOX_READY, @@ -180,20 +190,23 @@ func TestFilterSandboxes(t *testing.T) { }, expect: []*runtime.PodSandbox{testSandboxes[0], testSandboxes[2]}, }, - "label filter": { + { + desc: "label filter", filter: &runtime.PodSandboxFilter{ LabelSelector: map[string]string{"a": "b"}, }, expect: []*runtime.PodSandbox{testSandboxes[1]}, }, - "mixed filter not matched": { + { + desc: "mixed filter not matched", filter: &runtime.PodSandboxFilter{ Id: "1", LabelSelector: map[string]string{"a": "b"}, }, expect: []*runtime.PodSandbox{}, }, - "mixed filter matched": { + { + desc: "mixed filter matched", filter: &runtime.PodSandboxFilter{ State: &runtime.PodSandboxStateValue{ State: runtime.PodSandboxState_SANDBOX_READY, @@ -203,9 +216,10 @@ func TestFilterSandboxes(t *testing.T) { expect: []*runtime.PodSandbox{testSandboxes[2]}, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { filtered := c.filterCRISandboxes(testSandboxes, test.filter) - assert.Equal(t, test.expect, filtered, desc) + assert.Equal(t, test.expect, filtered, test.desc) }) } } diff --git a/pkg/cri/server/sandbox_run_linux_test.go b/pkg/cri/server/sandbox_run_linux_test.go index 70209a45d..a7f53a514 100644 --- a/pkg/cri/server/sandbox_run_linux_test.go +++ b/pkg/cri/server/sandbox_run_linux_test.go @@ -117,12 +117,14 @@ func TestLinuxSandboxContainerSpec(t *testing.T) { Size: 10, } - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string configChange func(*runtime.PodSandboxConfig) specCheck func(*testing.T, *runtimespec.Spec) expectErr bool }{ - "spec should reflect original config": { + { + desc: "spec should reflect original config", specCheck: func(t *testing.T, spec *runtimespec.Spec) { // runtime spec should have expected namespaces enabled by default. require.NotNil(t, spec.Linux) @@ -146,7 +148,8 @@ func TestLinuxSandboxContainerSpec(t *testing.T) { }) }, }, - "host namespace": { + { + desc: "host namespace", configChange: func(c *runtime.PodSandboxConfig) { c.Linux.SecurityContext = &runtime.LinuxSandboxSecurityContext{ NamespaceOptions: &runtime.NamespaceOption{ @@ -178,7 +181,8 @@ func TestLinuxSandboxContainerSpec(t *testing.T) { assert.NotContains(t, spec.Linux.Sysctl["net.ipv4.ping_group_range"], "0 2147483647") }, }, - "user namespace": { + { + desc: "user namespace", configChange: func(c *runtime.PodSandboxConfig) { c.Linux.SecurityContext = &runtime.LinuxSandboxSecurityContext{ NamespaceOptions: &runtime.NamespaceOption{ @@ -200,7 +204,8 @@ func TestLinuxSandboxContainerSpec(t *testing.T) { }, }, - "user namespace mode node and mappings": { + { + desc: "user namespace mode node and mappings", configChange: func(c *runtime.PodSandboxConfig) { c.Linux.SecurityContext = &runtime.LinuxSandboxSecurityContext{ NamespaceOptions: &runtime.NamespaceOption{ @@ -214,7 +219,8 @@ func TestLinuxSandboxContainerSpec(t *testing.T) { }, expectErr: true, }, - "user namespace with several mappings": { + { + desc: "user namespace with several mappings", configChange: func(c *runtime.PodSandboxConfig) { c.Linux.SecurityContext = &runtime.LinuxSandboxSecurityContext{ NamespaceOptions: &runtime.NamespaceOption{ @@ -228,7 +234,8 @@ func TestLinuxSandboxContainerSpec(t *testing.T) { }, expectErr: true, }, - "user namespace with uneven mappings": { + { + desc: "user namespace with uneven mappings", configChange: func(c *runtime.PodSandboxConfig) { c.Linux.SecurityContext = &runtime.LinuxSandboxSecurityContext{ NamespaceOptions: &runtime.NamespaceOption{ @@ -242,7 +249,8 @@ func TestLinuxSandboxContainerSpec(t *testing.T) { }, expectErr: true, }, - "user namespace mode container": { + { + desc: "user namespace mode container", configChange: func(c *runtime.PodSandboxConfig) { c.Linux.SecurityContext = &runtime.LinuxSandboxSecurityContext{ NamespaceOptions: &runtime.NamespaceOption{ @@ -254,7 +262,8 @@ func TestLinuxSandboxContainerSpec(t *testing.T) { }, expectErr: true, }, - "user namespace mode target": { + { + desc: "user namespace mode target", configChange: func(c *runtime.PodSandboxConfig) { c.Linux.SecurityContext = &runtime.LinuxSandboxSecurityContext{ NamespaceOptions: &runtime.NamespaceOption{ @@ -266,7 +275,8 @@ func TestLinuxSandboxContainerSpec(t *testing.T) { }, expectErr: true, }, - "user namespace unknown mode": { + { + desc: "user namespace unknown mode", configChange: func(c *runtime.PodSandboxConfig) { c.Linux.SecurityContext = &runtime.LinuxSandboxSecurityContext{ NamespaceOptions: &runtime.NamespaceOption{ @@ -278,7 +288,8 @@ func TestLinuxSandboxContainerSpec(t *testing.T) { }, expectErr: true, }, - "should set supplemental groups correctly": { + { + desc: "should set supplemental groups correctly", configChange: func(c *runtime.PodSandboxConfig) { c.Linux.SecurityContext = &runtime.LinuxSandboxSecurityContext{ SupplementalGroups: []int64{1111, 2222}, @@ -290,7 +301,8 @@ func TestLinuxSandboxContainerSpec(t *testing.T) { assert.Contains(t, spec.Process.User.AdditionalGids, uint32(2222)) }, }, - "should overwrite default sysctls": { + { + desc: "should overwrite default sysctls", configChange: func(c *runtime.PodSandboxConfig) { c.Linux.Sysctls = map[string]string{ "net.ipv4.ip_unprivileged_port_start": "500", @@ -303,7 +315,8 @@ func TestLinuxSandboxContainerSpec(t *testing.T) { assert.Contains(t, spec.Linux.Sysctl["net.ipv4.ping_group_range"], "1 1000") }, }, - "sandbox sizing annotations should be set if LinuxContainerResources were provided": { + { + desc: "sandbox sizing annotations should be set if LinuxContainerResources were provided", configChange: func(c *runtime.PodSandboxConfig) { c.Linux.Resources = &v1.LinuxContainerResources{ CpuPeriod: 100, @@ -331,7 +344,8 @@ func TestLinuxSandboxContainerSpec(t *testing.T) { assert.EqualValues(t, "1024", value) }, }, - "sandbox sizing annotations should not be set if LinuxContainerResources were not provided": { + { + desc: "sandbox sizing annotations should not be set if LinuxContainerResources were not provided", specCheck: func(t *testing.T, spec *runtimespec.Spec) { _, ok := spec.Annotations[annotations.SandboxCPUPeriod] assert.False(t, ok) @@ -343,7 +357,8 @@ func TestLinuxSandboxContainerSpec(t *testing.T) { assert.False(t, ok) }, }, - "sandbox sizing annotations are zero if the resources are set to 0": { + { + desc: "sandbox sizing annotations are zero if the resources are set to 0", configChange: func(c *runtime.PodSandboxConfig) { c.Linux.Resources = &v1.LinuxContainerResources{} }, @@ -363,7 +378,8 @@ func TestLinuxSandboxContainerSpec(t *testing.T) { }, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { c := newTestCRIService() c.config.EnableUnprivilegedICMP = true c.config.EnableUnprivilegedPorts = true @@ -392,13 +408,15 @@ func TestSetupSandboxFiles(t *testing.T) { testID = "test-id" realhostname = "test-real-hostname" ) - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string dnsConfig *runtime.DNSConfig hostname string ipcMode runtime.NamespaceMode expectedCalls []ostesting.CalledDetail }{ - "should check host /dev/shm existence when ipc mode is NODE": { + { + desc: "should check host /dev/shm existence when ipc mode is NODE", ipcMode: runtime.NamespaceMode_NODE, expectedCalls: []ostesting.CalledDetail{ { @@ -434,7 +452,8 @@ func TestSetupSandboxFiles(t *testing.T) { }, }, }, - "should create new /etc/resolv.conf if DNSOptions is set": { + { + desc: "should create new /etc/resolv.conf if DNSOptions is set", dnsConfig: &runtime.DNSConfig{ Servers: []string{"8.8.8.8"}, Searches: []string{"114.114.114.114"}, @@ -477,7 +496,8 @@ options timeout:1 }, }, }, - "should create sandbox shm when ipc namespace mode is not NODE": { + { + desc: "should create sandbox shm when ipc namespace mode is not NODE", ipcMode: runtime.NamespaceMode_POD, expectedCalls: []ostesting.CalledDetail{ { @@ -520,7 +540,8 @@ options timeout:1 }, }, }, - "should create /etc/hostname when hostname is set": { + { + desc: "should create /etc/hostname when hostname is set", hostname: "test-hostname", ipcMode: runtime.NamespaceMode_NODE, expectedCalls: []ostesting.CalledDetail{ @@ -555,7 +576,8 @@ options timeout:1 }, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { c := newTestCRIService() c.os.(*ostesting.FakeOS).HostnameFn = func() (string, error) { return realhostname, nil @@ -586,15 +608,19 @@ options timeout:1 } func TestParseDNSOption(t *testing.T) { - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string servers []string searches []string options []string expectedContent string expectErr bool }{ - "empty dns options should return empty content": {}, - "non-empty dns options should return correct content": { + { + desc: "empty dns options should return empty content", + }, + { + desc: "non-empty dns options should return correct content", servers: []string{"8.8.8.8", "server.google.com"}, searches: []string{"114.114.114.114"}, options: []string{"timeout:1"}, @@ -604,7 +630,8 @@ nameserver server.google.com options timeout:1 `, }, - "expanded dns config should return correct content on modern libc (e.g. glibc 2.26 and above)": { + { + desc: "expanded dns config should return correct content on modern libc (e.g. glibc 2.26 and above)", servers: []string{"8.8.8.8", "server.google.com"}, searches: []string{ "server0.google.com", @@ -623,7 +650,8 @@ options timeout:1 `, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { resolvContent, err := parseDNSOptions(test.servers, test.searches, test.options) if test.expectErr { assert.Error(t, err) diff --git a/pkg/cri/server/sandbox_run_test.go b/pkg/cri/server/sandbox_run_test.go index 26d71cad7..7070275bf 100644 --- a/pkg/cri/server/sandbox_run_test.go +++ b/pkg/cri/server/sandbox_run_test.go @@ -41,27 +41,31 @@ func TestSandboxContainerSpec(t *testing.T) { } testID := "test-id" nsPath := "test-cni" - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string configChange func(*runtime.PodSandboxConfig) podAnnotations []string imageConfigChange func(*imagespec.ImageConfig) specCheck func(*testing.T, *runtimespec.Spec) expectErr bool }{ - "should return error when entrypoint and cmd are empty": { + { + desc: "should return error when entrypoint and cmd are empty", imageConfigChange: func(c *imagespec.ImageConfig) { c.Entrypoint = nil c.Cmd = nil }, expectErr: true, }, - "a passthrough annotation should be passed as an OCI annotation": { + { + desc: "a passthrough annotation should be passed as an OCI annotation", podAnnotations: []string{"c"}, specCheck: func(t *testing.T, spec *runtimespec.Spec) { assert.Equal(t, spec.Annotations["c"], "d") }, }, - "a non-passthrough annotation should not be passed as an OCI annotation": { + { + desc: "a non-passthrough annotation should not be passed as an OCI annotation", configChange: func(c *runtime.PodSandboxConfig) { c.Annotations["d"] = "e" }, @@ -72,7 +76,8 @@ func TestSandboxContainerSpec(t *testing.T) { assert.False(t, ok) }, }, - "passthrough annotations should support wildcard match": { + { + desc: "passthrough annotations should support wildcard match", configChange: func(c *runtime.PodSandboxConfig) { c.Annotations["t.f"] = "j" c.Annotations["z.g"] = "o" @@ -92,7 +97,8 @@ func TestSandboxContainerSpec(t *testing.T) { }, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { c := newTestCRIService() config, imageConfig, specCheck := getRunPodSandboxTestData() if test.configChange != nil { @@ -120,11 +126,15 @@ func TestSandboxContainerSpec(t *testing.T) { } func TestTypeurlMarshalUnmarshalSandboxMeta(t *testing.T) { - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string configChange func(*runtime.PodSandboxConfig) }{ - "should marshal original config": {}, - "should marshal Linux": { + { + desc: "should marshal original config", + }, + { + desc: "should marshal Linux", configChange: func(c *runtime.PodSandboxConfig) { if c.Linux == nil { c.Linux = &runtime.LinuxPodSandboxConfig{} @@ -140,7 +150,8 @@ func TestTypeurlMarshalUnmarshalSandboxMeta(t *testing.T) { }, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { meta := &sandboxstore.Metadata{ ID: "1", Name: "sandbox_1", @@ -164,12 +175,16 @@ func TestTypeurlMarshalUnmarshalSandboxMeta(t *testing.T) { } func TestToCNIPortMappings(t *testing.T) { - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string criPortMappings []*runtime.PortMapping cniPortMappings []cni.PortMapping }{ - "empty CRI port mapping should map to empty CNI port mapping": {}, - "CRI port mapping should be converted to CNI port mapping properly": { + { + desc: "empty CRI port mapping should map to empty CNI port mapping", + }, + { + desc: "CRI port mapping should be converted to CNI port mapping properly", criPortMappings: []*runtime.PortMapping{ { Protocol: runtime.Protocol_UDP, @@ -211,7 +226,8 @@ func TestToCNIPortMappings(t *testing.T) { }, }, }, - "CRI port mapping without host port should be skipped": { + { + desc: "CRI port mapping without host port should be skipped", criPortMappings: []*runtime.PortMapping{ { Protocol: runtime.Protocol_UDP, @@ -234,7 +250,8 @@ func TestToCNIPortMappings(t *testing.T) { }, }, }, - "CRI port mapping with unsupported protocol should be skipped": { + { + desc: "CRI port mapping with unsupported protocol should be skipped", criPortMappings: []*runtime.PortMapping{ { Protocol: runtime.Protocol_TCP, @@ -253,54 +270,63 @@ func TestToCNIPortMappings(t *testing.T) { }, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { assert.Equal(t, test.cniPortMappings, toCNIPortMappings(test.criPortMappings)) }) } } func TestSelectPodIP(t *testing.T) { - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string ips []string expectedIP string expectedAdditionalIPs []string pref string }{ - "ipv4 should be picked even if ipv6 comes first": { + { + desc: "ipv4 should be picked even if ipv6 comes first", ips: []string{"2001:db8:85a3::8a2e:370:7334", "192.168.17.43"}, expectedIP: "192.168.17.43", expectedAdditionalIPs: []string{"2001:db8:85a3::8a2e:370:7334"}, }, - "ipv6 should be picked even if ipv4 comes first": { + { + desc: "ipv6 should be picked even if ipv4 comes first", ips: []string{"2001:db8:85a3::8a2e:370:7334", "192.168.17.43"}, expectedIP: "2001:db8:85a3::8a2e:370:7334", expectedAdditionalIPs: []string{"192.168.17.43"}, pref: "ipv6", }, - "order should reflect ip selection": { + { + desc: "order should reflect ip selection", ips: []string{"2001:db8:85a3::8a2e:370:7334", "192.168.17.43"}, expectedIP: "2001:db8:85a3::8a2e:370:7334", expectedAdditionalIPs: []string{"192.168.17.43"}, pref: "cni", }, - "ipv4 should be picked when there is only ipv4": { + { + desc: "ipv4 should be picked when there is only ipv4", ips: []string{"192.168.17.43"}, expectedIP: "192.168.17.43", expectedAdditionalIPs: nil, }, - "ipv6 should be picked when there is no ipv4": { + { + desc: "ipv6 should be picked when there is no ipv4", ips: []string{"2001:db8:85a3::8a2e:370:7334"}, expectedIP: "2001:db8:85a3::8a2e:370:7334", expectedAdditionalIPs: nil, }, - "the first ipv4 should be picked when there are multiple ipv4": { // unlikely to happen + { + desc: "the first ipv4 should be picked when there are multiple ipv4", // unlikely to happen ips: []string{"2001:db8:85a3::8a2e:370:7334", "192.168.17.43", "2001:db8:85a3::8a2e:370:7335", "192.168.17.45"}, expectedIP: "192.168.17.43", expectedAdditionalIPs: []string{"2001:db8:85a3::8a2e:370:7334", "2001:db8:85a3::8a2e:370:7335", "192.168.17.45"}, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { var ipConfigs []*cni.IPConfig for _, ip := range test.ips { ipConfigs = append(ipConfigs, &cni.IPConfig{ diff --git a/pkg/cri/server/sandbox_stats_windows_test.go b/pkg/cri/server/sandbox_stats_windows_test.go index 8e6436e2b..40bb82d7a 100644 --- a/pkg/cri/server/sandbox_stats_windows_test.go +++ b/pkg/cri/server/sandbox_stats_windows_test.go @@ -33,20 +33,23 @@ func TestGetUsageNanoCores(t *testing.T) { secondAfterTimeStamp := timestamp.Add(time.Second) ID := "ID" - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string firstCPUValue uint64 secondCPUValue uint64 expectedNanoCoreUsageFirst uint64 expectedNanoCoreUsageSecond uint64 }{ - "metrics": { + { + desc: "metrics", firstCPUValue: 50, secondCPUValue: 500, expectedNanoCoreUsageFirst: 0, expectedNanoCoreUsageSecond: 450, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { container, err := containerstore.NewContainer( containerstore.Metadata{ID: ID}, ) @@ -85,7 +88,8 @@ func Test_criService_podSandboxStats(t *testing.T) { UsageNanoCores uint64 WorkingSetBytes uint64 } - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string metrics map[string]*wstats.Statistics sandbox sandboxstore.Sandbox containers []containerstore.Container @@ -93,7 +97,8 @@ func Test_criService_podSandboxStats(t *testing.T) { expectedContainerStats []expectedStats expectError bool }{ - "no metrics found should return error": { + { + desc: "no metrics found should return error", metrics: map[string]*wstats.Statistics{}, sandbox: sandboxstore.Sandbox{}, containers: []containerstore.Container{}, @@ -101,7 +106,8 @@ func Test_criService_podSandboxStats(t *testing.T) { expectedContainerStats: []expectedStats{}, expectError: true, }, - "pod stats will include the container stats": { + { + desc: "pod stats will include the container stats", metrics: map[string]*wstats.Statistics{ "c1": { Container: windowsStat(currentStatsTimestamp, 200, 20), @@ -128,7 +134,8 @@ func Test_criService_podSandboxStats(t *testing.T) { }, expectError: false, }, - "pod with existing stats will have usagenanocores totalled across pods and containers": { + { + desc: "pod with existing stats will have usagenanocores totalled across pods and containers", metrics: map[string]*wstats.Statistics{ "c1": { Container: windowsStat(currentStatsTimestamp, 400, 20), @@ -161,7 +168,8 @@ func Test_criService_podSandboxStats(t *testing.T) { }, expectError: false, }, - "pod sandbox with nil stats still works (hostprocess container scenario)": { + { + desc: "pod sandbox with nil stats still works (hostprocess container scenario)", metrics: map[string]*wstats.Statistics{ "c1": { Container: windowsStat(currentStatsTimestamp, 400, 20), @@ -193,7 +201,8 @@ func Test_criService_podSandboxStats(t *testing.T) { expectError: false, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { actualPodStats, actualContainerStats, err := c.toPodSandboxStats(test.sandbox, test.metrics, test.containers, currentStatsTimestamp) if test.expectError { assert.NotNil(t, err) @@ -240,25 +249,29 @@ func Test_criService_saveSandBoxMetrics(t *testing.T) { timestamp := time.Now() containerID := "c1" sandboxID := "s1" - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string sandboxStats *runtime.PodSandboxStats expectError bool expectedSandboxvalue *stats.ContainerStats expectedContainervalue *stats.ContainerStats }{ - "if sandboxstats is nil then skip ": { + { + desc: "if sandboxstats is nil then skip ", sandboxStats: nil, expectError: false, expectedSandboxvalue: nil, }, - "if sandboxstats.windows is nil then skip": { + { + desc: "if sandboxstats.windows is nil then skip", sandboxStats: &runtime.PodSandboxStats{ Windows: nil, }, expectError: false, expectedSandboxvalue: nil, }, - "if sandboxstats.windows.cpu is nil then skip": { + { + desc: "if sandboxstats.windows.cpu is nil then skip", sandboxStats: &runtime.PodSandboxStats{ Windows: &runtime.WindowsPodSandboxStats{ Cpu: nil, @@ -267,7 +280,8 @@ func Test_criService_saveSandBoxMetrics(t *testing.T) { expectError: false, expectedSandboxvalue: nil, }, - "if sandboxstats.windows.cpu.UsageCoreNanoSeconds is nil then skip": { + { + desc: "if sandboxstats.windows.cpu.UsageCoreNanoSeconds is nil then skip", sandboxStats: &runtime.PodSandboxStats{ Windows: &runtime.WindowsPodSandboxStats{ Cpu: &runtime.WindowsCpuUsage{ @@ -278,7 +292,8 @@ func Test_criService_saveSandBoxMetrics(t *testing.T) { expectError: false, expectedSandboxvalue: nil, }, - "Stats for containers that have cpu nil are skipped": { + { + desc: "Stats for containers that have cpu nil are skipped", sandboxStats: &runtime.PodSandboxStats{ Windows: &runtime.WindowsPodSandboxStats{ Cpu: &runtime.WindowsCpuUsage{ @@ -300,7 +315,8 @@ func Test_criService_saveSandBoxMetrics(t *testing.T) { }, expectedContainervalue: nil, }, - "Stats for containers that have UsageCoreNanoSeconds nil are skipped": { + { + desc: "Stats for containers that have UsageCoreNanoSeconds nil are skipped", sandboxStats: &runtime.PodSandboxStats{ Windows: &runtime.WindowsPodSandboxStats{ Cpu: &runtime.WindowsCpuUsage{ @@ -324,7 +340,8 @@ func Test_criService_saveSandBoxMetrics(t *testing.T) { }, expectedContainervalue: nil, }, - "Stats are updated for sandbox and containers": { + { + desc: "Stats are updated for sandbox and containers", sandboxStats: &runtime.PodSandboxStats{ Windows: &runtime.WindowsPodSandboxStats{ Cpu: &runtime.WindowsCpuUsage{ @@ -353,7 +370,8 @@ func Test_criService_saveSandBoxMetrics(t *testing.T) { }, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { c := newTestCRIService() c.sandboxStore.Add(sandboxstore.Sandbox{ Metadata: sandboxstore.Metadata{ID: sandboxID}, diff --git a/pkg/cri/server/sandbox_status_test.go b/pkg/cri/server/sandbox_status_test.go index 273c61531..61eaacfeb 100644 --- a/pkg/cri/server/sandbox_status_test.go +++ b/pkg/cri/server/sandbox_status_test.go @@ -87,24 +87,29 @@ func TestPodSandboxStatus(t *testing.T) { Annotations: config.GetAnnotations(), RuntimeHandler: "test-runtime-handler", } - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string state sandboxstore.State expectedState runtime.PodSandboxState }{ - "sandbox state ready": { + { + desc: "sandbox state ready", state: sandboxstore.StateReady, expectedState: runtime.PodSandboxState_SANDBOX_READY, }, - "sandbox state not ready": { + { + desc: "sandbox state not ready", state: sandboxstore.StateNotReady, expectedState: runtime.PodSandboxState_SANDBOX_NOTREADY, }, - "sandbox state unknown": { + { + desc: "sandbox state unknown", state: sandboxstore.StateUnknown, expectedState: runtime.PodSandboxState_SANDBOX_NOTREADY, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { status := sandboxstore.Status{ CreatedAt: createdAt, State: test.state, diff --git a/pkg/cri/server/sandbox_stop_test.go b/pkg/cri/server/sandbox_stop_test.go index 089fdebb2..e58bc58b9 100644 --- a/pkg/cri/server/sandbox_stop_test.go +++ b/pkg/cri/server/sandbox_stop_test.go @@ -28,30 +28,35 @@ import ( func TestWaitSandboxStop(t *testing.T) { id := "test-id" - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string state sandboxstore.State cancel bool timeout time.Duration expectErr bool }{ - "should return error if timeout exceeds": { + { + desc: "should return error if timeout exceeds", state: sandboxstore.StateReady, timeout: 200 * time.Millisecond, expectErr: true, }, - "should return error if context is cancelled": { + { + desc: "should return error if context is cancelled", state: sandboxstore.StateReady, timeout: time.Hour, cancel: true, expectErr: true, }, - "should not return error if sandbox is stopped before timeout": { + { + desc: "should not return error if sandbox is stopped before timeout", state: sandboxstore.StateNotReady, timeout: time.Hour, expectErr: false, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { c := newTestCRIService() sandbox := sandboxstore.NewSandbox( sandboxstore.Metadata{ID: id}, @@ -69,7 +74,7 @@ func TestWaitSandboxStop(t *testing.T) { ctx = timeoutCtx } err := c.waitSandboxStop(ctx, sandbox) - assert.Equal(t, test.expectErr, err != nil, desc) + assert.Equal(t, test.expectErr, err != nil, test.desc) }) } } diff --git a/pkg/cri/server/streaming_test.go b/pkg/cri/server/streaming_test.go index 32db53261..d7b7b427e 100644 --- a/pkg/cri/server/streaming_test.go +++ b/pkg/cri/server/streaming_test.go @@ -24,12 +24,14 @@ import ( ) func TestValidateStreamServer(t *testing.T) { - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string *criService tlsMode streamListenerMode expectErr bool }{ - "should pass with default withoutTLS": { + { + desc: "should pass with default withoutTLS", criService: &criService{ config: config.Config{ PluginConfig: config.DefaultConfig(), @@ -38,7 +40,8 @@ func TestValidateStreamServer(t *testing.T) { tlsMode: withoutTLS, expectErr: false, }, - "should pass with x509KeyPairTLS": { + { + desc: "should pass with x509KeyPairTLS", criService: &criService{ config: config.Config{ PluginConfig: config.PluginConfig{ @@ -53,7 +56,8 @@ func TestValidateStreamServer(t *testing.T) { tlsMode: x509KeyPairTLS, expectErr: false, }, - "should pass with selfSign": { + { + desc: "should pass with selfSign", criService: &criService{ config: config.Config{ PluginConfig: config.PluginConfig{ @@ -64,7 +68,8 @@ func TestValidateStreamServer(t *testing.T) { tlsMode: selfSignTLS, expectErr: false, }, - "should return error with X509 keypair but not EnableTLSStreaming": { + { + desc: "should return error with X509 keypair but not EnableTLSStreaming", criService: &criService{ config: config.Config{ PluginConfig: config.PluginConfig{ @@ -79,7 +84,8 @@ func TestValidateStreamServer(t *testing.T) { tlsMode: -1, expectErr: true, }, - "should return error with X509 TLSCertFile empty": { + { + desc: "should return error with X509 TLSCertFile empty", criService: &criService{ config: config.Config{ PluginConfig: config.PluginConfig{ @@ -94,7 +100,8 @@ func TestValidateStreamServer(t *testing.T) { tlsMode: -1, expectErr: true, }, - "should return error with X509 TLSKeyFile empty": { + { + desc: "should return error with X509 TLSKeyFile empty", criService: &criService{ config: config.Config{ PluginConfig: config.PluginConfig{ @@ -109,7 +116,8 @@ func TestValidateStreamServer(t *testing.T) { tlsMode: -1, expectErr: true, }, - "should return error without EnableTLSStreaming and only TLSCertFile set": { + { + desc: "should return error without EnableTLSStreaming and only TLSCertFile set", criService: &criService{ config: config.Config{ PluginConfig: config.PluginConfig{ @@ -124,7 +132,8 @@ func TestValidateStreamServer(t *testing.T) { tlsMode: -1, expectErr: true, }, - "should return error without EnableTLSStreaming and only TLSKeyFile set": { + { + desc: "should return error without EnableTLSStreaming and only TLSKeyFile set", criService: &criService{ config: config.Config{ PluginConfig: config.PluginConfig{ @@ -140,7 +149,8 @@ func TestValidateStreamServer(t *testing.T) { expectErr: true, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { tlsMode, err := getStreamListenerMode(test.criService) if test.expectErr { assert.Error(t, err) diff --git a/pkg/cri/server/update_runtime_config_test.go b/pkg/cri/server/update_runtime_config_test.go index 6cb535ca4..bed07902b 100644 --- a/pkg/cri/server/update_runtime_config_test.go +++ b/pkg/cri/server/update_runtime_config_test.go @@ -70,29 +70,35 @@ func TestUpdateRuntimeConfig(t *testing.T) { }` ) - for name, test := range map[string]struct { + for _, test := range []struct { + name string noTemplate bool emptyCIDR bool networkReady bool expectCNIConfig bool }{ - "should not generate cni config if cidr is empty": { + { + name: "should not generate cni config if cidr is empty", emptyCIDR: true, expectCNIConfig: false, }, - "should not generate cni config if template file is not specified": { + { + name: "should not generate cni config if template file is not specified", noTemplate: true, expectCNIConfig: false, }, - "should not generate cni config if network is ready": { + { + name: "should not generate cni config if network is ready", networkReady: true, expectCNIConfig: false, }, - "should generate cni config if template is specified and cidr is provided": { + { + name: "should generate cni config if template is specified and cidr is provided", expectCNIConfig: true, }, } { - t.Run(name, func(t *testing.T) { + test := test + t.Run(test.name, func(t *testing.T) { testDir := t.TempDir() templateName := filepath.Join(testDir, "template") err := os.WriteFile(templateName, []byte(testTemplate), 0666)