github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-22 19:31:44 +00:00
Code Issues Projects Releases Wiki Activity

Promote PodOS field to GA

Browse Source
This commit is contained in:
Ravi Gudimetla 2022-07-18 12:47:19 -04:00
parent 5108b0a3a0
commit b79ebb8165
5 changed files with 83 additions and 349 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
pkg/api/pod/util.go
View File

@ -418,8 +418,6 @@ func GetValidationOptionsFromPodSpecAndMeta(podSpec, oldPodSpec *api.PodSpec, po
AllowWindowsHostProcessField: utilfeature.DefaultFeatureGate.Enabled(features.WindowsHostProcessContainers),
// Allow pod spec with expanded DNS configuration
AllowExpandedDNSConfig: utilfeature.DefaultFeatureGate.Enabled(features.ExpandedDNSConfig) || haveSameExpandedDNSConfig(podSpec, oldPodSpec),
// Allow pod spec to use OS field
AllowOSField: utilfeature.DefaultFeatureGate.Enabled(features.IdentifyPodOS),
}
if oldPodSpec != nil {
@ -435,9 +433,6 @@ func GetValidationOptionsFromPodSpecAndMeta(podSpec, oldPodSpec *api.PodSpec, po
// if old spec has Windows Host Process fields set, we must allow it
opts.AllowWindowsHostProcessField = opts.AllowWindowsHostProcessField || setsWindowsHostProcess(oldPodSpec)
// if old spec has OS field set, we must allow it
opts.AllowOSField = opts.AllowOSField || oldPodSpec.OS != nil
// if old spec used non-integer multiple of huge page unit size, we must allow it
opts.AllowIndivisibleHugePagesValues = usesIndivisibleHugePagesValues(oldPodSpec)
@ -556,10 +551,6 @@ func dropDisabledFields(
dropDisabledCSIVolumeSourceAlphaFields(podSpec, oldPodSpec)
if !utilfeature.DefaultFeatureGate.Enabled(features.IdentifyPodOS) && !podOSInUse(oldPodSpec) {
podSpec.OS = nil
}
dropDisabledTopologySpreadConstraintsFields(podSpec, oldPodSpec)
dropDisabledNodeInclusionPolicyFields(podSpec, oldPodSpec)
}
@ -591,17 +582,6 @@ func minDomainsInUse(podSpec *api.PodSpec) bool {
return false
}
// podOSInUse returns true if the pod spec is non-nil and has OS field set
func podOSInUse(podSpec *api.PodSpec) bool {
if podSpec == nil {
return false
}
if podSpec.OS != nil {
return true
}
return false
}
// dropDisabledProcMountField removes disabled fields from PodSpec related
// to ProcMount only if it is not already used by the old spec
func dropDisabledProcMountField(podSpec, oldPodSpec *api.PodSpec) {

82
pkg/api/pod/util_test.go
View File

@ -1687,88 +1687,6 @@ func TestDropDisabledTopologySpreadConstraintsFields(t *testing.T) {
}
}
func TestDropOSField(t *testing.T) {
podWithOSField := func() *api.Pod {
osField := api.PodOS{Name: "linux"}
return &api.Pod{
Spec: api.PodSpec{
OS: &osField,
},
}
}
podWithoutOSField := func() *api.Pod { return &api.Pod{} }
podInfo := []struct {
description string
hasPodOSField bool
pod func() *api.Pod
}{
{
description: "has PodOS field",
hasPodOSField: true,
pod: podWithOSField,
},
{
description: "does not have PodOS field",
hasPodOSField: false,
pod: podWithoutOSField,
},
{
description: "is nil",
hasPodOSField: false,
pod: func() *api.Pod { return nil },
},
}
for _, enabled := range []bool{true, false} {
for _, oldPodInfo := range podInfo {
for _, newPodInfo := range podInfo {
oldPodHasOsField, oldPod := oldPodInfo.hasPodOSField, oldPodInfo.pod()
newPodHasOSField, newPod := newPodInfo.hasPodOSField, newPodInfo.pod()
if newPod == nil {
continue
}
t.Run(fmt.Sprintf("feature enabled=%v, old pod %v, new pod %v", enabled, oldPodInfo.description, newPodInfo.description), func(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.IdentifyPodOS, enabled)()
var oldPodSpec *api.PodSpec
if oldPod != nil {
oldPodSpec = &oldPod.Spec
}
dropDisabledFields(&newPod.Spec, nil, oldPodSpec, nil)
// old pod should never be changed
if !reflect.DeepEqual(oldPod, oldPodInfo.pod()) {
t.Errorf("old pod changed: %v", cmp.Diff(oldPod, oldPodInfo.pod()))
}
switch {
case enabled || oldPodHasOsField:
// new pod should not be changed if the feature is enabled, or if the old pod had subpaths
if !reflect.DeepEqual(newPod, newPodInfo.pod()) {
t.Errorf("new pod changed: %v", cmp.Diff(newPod, newPodInfo.pod()))
}
case newPodHasOSField:
// new pod should be changed
if reflect.DeepEqual(newPod, newPodInfo.pod()) {
t.Errorf("new pod was not changed")
}
// new pod should not have OSfield
if !reflect.DeepEqual(newPod, podWithoutOSField()) {
t.Errorf("new pod has OS field: %v", cmp.Diff(newPod, podWithoutOSField()))
}
default:
// new pod should not need to be changed
if !reflect.DeepEqual(newPod, newPodInfo.pod()) {
t.Errorf("new pod changed: %v", cmp.Diff(newPod, newPodInfo.pod()))
}
}
})
}
}
}
}
func TestDropNodeInclusionPolicyFields(t *testing.T) {
ignore := api.NodeInclusionPolicyIgnore
honor := api.NodeInclusionPolicyHonor

5
pkg/apis/core/validation/validation.go
View File

@ -3396,8 +3396,6 @@ type PodValidationOptions struct {
AllowWindowsHostProcessField bool
// Allow more DNSSearchPaths and longer DNSSearchListChars
AllowExpandedDNSConfig bool
// Allow OSField to be set in the pod spec
AllowOSField bool
}
// validatePodMetadataAndSpec tests if required fields in the pod.metadata and pod.spec are set,
@ -6345,9 +6343,6 @@ func validateOS(podSpec *core.PodSpec, fldPath *field.Path, opts PodValidationOp
if os == nil {
return allErrs
}
if !opts.AllowOSField {
return append(allErrs, field.Forbidden(fldPath, "cannot be set when IdentifyPodOS feature is not enabled"))
}
if len(os.Name) == 0 {
return append(allErrs, field.Required(fldPath.Child("name"), "cannot be empty"))
}

164
pkg/apis/core/validation/validation_test.go
View File

@ -6855,36 +6855,20 @@ func TestValidateWindowsPodSecurityContext(t *testing.T) {
expectErr bool
errorType field.ErrorType
errorDetail string
featureEnabled bool
}{
"valid SC, windows, no error": {
podSec: &core.PodSpec{SecurityContext: validWindowsSC},
expectErr: false,
featureEnabled: true,
},
"invalid SC, windows, error": {
podSec: &core.PodSpec{SecurityContext: invalidWindowsSC},
errorType: "FieldValueForbidden",
errorDetail: "cannot be set for a windows pod",
expectErr: true,
featureEnabled: true,
},
"valid SC, windows, no error, no IdentifyPodOS featuregate": {
podSec: &core.PodSpec{SecurityContext: validWindowsSC},
expectErr: false,
featureEnabled: false,
},
"invalid SC, windows, error, no IdentifyPodOS featuregate": {
podSec: &core.PodSpec{SecurityContext: invalidWindowsSC},
errorType: "FieldValueForbidden",
errorDetail: "cannot be set for a windows pod",
expectErr: true,
featureEnabled: false,
},
}
for k, v := range cases {
t.Run(k, func(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.IdentifyPodOS, v.featureEnabled)()
errs := validateWindows(v.podSec, field.NewPath("field"))
if v.expectErr && len(errs) > 0 {
if errs[0].Type != v.errorType || !strings.Contains(errs[0].Detail, v.errorDetail) {
@ -6920,36 +6904,20 @@ func TestValidateLinuxPodSecurityContext(t *testing.T) {
expectErr bool
errorType field.ErrorType
errorDetail string
featureEnabled bool
}{
"valid SC, linux, no error": {
podSpec: &core.PodSpec{SecurityContext: validLinuxSC},
expectErr: false,
featureEnabled: true,
},
"invalid SC, linux, error": {
podSpec: &core.PodSpec{SecurityContext: invalidLinuxSC},
errorType: "FieldValueForbidden",
errorDetail: "windows options cannot be set for a linux pod",
expectErr: true,
featureEnabled: true,
},
"valid SC, linux, no error, no IdentifyPodOS featuregate": {
podSpec: &core.PodSpec{SecurityContext: validLinuxSC},
expectErr: false,
featureEnabled: false,
},
"invalid SC, linux, error, no IdentifyPodOS featuregate": {
podSpec: &core.PodSpec{SecurityContext: invalidLinuxSC},
errorType: "FieldValueForbidden",
errorDetail: "windows options cannot be set for a linux pod",
expectErr: true,
featureEnabled: false,
},
}
for k, v := range cases {
t.Run(k, func(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.IdentifyPodOS, v.featureEnabled)()
errs := validateLinux(v.podSpec, field.NewPath("field"))
if v.expectErr && len(errs) > 0 {
if errs[0].Type != v.errorType || !strings.Contains(errs[0].Detail, v.errorDetail) {
@ -9968,7 +9936,6 @@ func TestValidatePodUpdate(t *testing.T) {
old core.Pod
err string
test string
enablePodOS bool
}{
{new: core.Pod{}, old: core.Pod{}, err: "", test: "nothing"},
{
@ -10773,29 +10740,6 @@ func TestValidatePodUpdate(t *testing.T) {
err: "spec: Forbidden: pod updates",
test: "update termination grace period seconds not 1",
},
{
new: core.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
},
Spec: core.PodSpec{
OS: &core.PodOS{Name: core.Windows},
SecurityContext: &core.PodSecurityContext{SELinuxOptions: &core.SELinuxOptions{Role: "dummy"}},
},
},
old: core.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
},
Spec: core.PodSpec{
OS: &core.PodOS{Name: core.Linux},
SecurityContext: &core.PodSecurityContext{SELinuxOptions: &core.SELinuxOptions{Role: "dummy"}},
},
},
err: "Forbidden: pod updates may not change fields other than `spec.containers[*].image`,",
test: "pod OS changing from Linux to Windows, no IdentifyPodOS featuregate set, no validation done",
enablePodOS: false,
},
{
new: core.Pod{
ObjectMeta: metav1.ObjectMeta{
@ -10817,7 +10761,6 @@ func TestValidatePodUpdate(t *testing.T) {
},
err: "Forbidden: pod updates may not change fields other than `spec.containers[*].image",
test: "pod OS changing from Linux to Windows, IdentifyPodOS featuregate set",
enablePodOS: true,
},
{
new: core.Pod{
@ -10840,7 +10783,6 @@ func TestValidatePodUpdate(t *testing.T) {
},
err: "spec.securityContext.seLinuxOptions: Forbidden",
test: "pod OS changing from Linux to Windows, IdentifyPodOS featuregate set, we'd get SELinux errors as well",
enablePodOS: true,
},
{
new: core.Pod{
@ -10859,26 +10801,6 @@ func TestValidatePodUpdate(t *testing.T) {
},
err: "Forbidden: pod updates may not change fields other than `spec.containers[*].image",
test: "invalid PodOS update, IdentifyPodOS featuregate set",
enablePodOS: true,
},
{
new: core.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
},
Spec: core.PodSpec{
OS: &core.PodOS{Name: core.Windows},
},
},
old: core.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
},
Spec: core.PodSpec{},
},
err: "Forbidden: pod updates may not change fields other than `spec.containers[*].image",
test: "no pod spec OS to a valid value, no featuregate",
enablePodOS: false,
},
{
new: core.Pod{
@ -10927,7 +10849,7 @@ func TestValidatePodUpdate(t *testing.T) {
test.old.Spec.RestartPolicy = "Always"
}
errs := ValidatePodUpdate(&test.new, &test.old, PodValidationOptions{AllowOSField: test.enablePodOS})
errs := ValidatePodUpdate(&test.new, &test.old, PodValidationOptions{})
if test.err == "" {
if len(errs) != 0 {
t.Errorf("unexpected invalid: %s (%+v)\nA: %+v\nB: %+v", test.test, errs, test.new, test.old)
@ -17509,7 +17431,6 @@ func TestValidateWindowsSecurityContext(t *testing.T) {
expectError bool
errorMsg string
errorType field.ErrorType
featureEnabled bool
}{
{
name: "pod with SELinux Options",
@ -17517,7 +17438,6 @@ func TestValidateWindowsSecurityContext(t *testing.T) {
expectError: true,
errorMsg: "cannot be set for a windows pod",
errorType: "FieldValueForbidden",
featureEnabled: true,
},
{
name: "pod with SeccompProfile",
@ -17525,40 +17445,15 @@ func TestValidateWindowsSecurityContext(t *testing.T) {
expectError: true,
errorMsg: "cannot be set for a windows pod",
errorType: "FieldValueForbidden",
featureEnabled: true,
},
{
name: "pod with WindowsOptions, no error",
sc: &core.PodSpec{Containers: []core.Container{{SecurityContext: &core.SecurityContext{WindowsOptions: &core.WindowsSecurityContextOptions{RunAsUserName: utilpointer.String("dummy")}}}}},
expectError: false,
featureEnabled: true,
},
{
name: "pod with SELinux Options, no IdentifyPodOS enabled",
sc: &core.PodSpec{Containers: []core.Container{{SecurityContext: &core.SecurityContext{SELinuxOptions: &core.SELinuxOptions{Role: "dummy"}}}}},
expectError: true,
errorMsg: "cannot be set for a windows pod",
errorType: "FieldValueForbidden",
featureEnabled: false,
},
{
name: "pod with SeccompProfile, no IdentifyPodOS enabled",
sc: &core.PodSpec{Containers: []core.Container{{SecurityContext: &core.SecurityContext{SeccompProfile: &core.SeccompProfile{LocalhostProfile: utilpointer.String("dummy")}}}}},
expectError: true,
errorMsg: "cannot be set for a windows pod",
errorType: "FieldValueForbidden",
featureEnabled: false,
},
{
name: "pod with WindowsOptions, no error, no IdentifyPodOS enabled",
sc: &core.PodSpec{Containers: []core.Container{{SecurityContext: &core.SecurityContext{WindowsOptions: &core.WindowsSecurityContextOptions{RunAsUserName: utilpointer.String("dummy")}}}}},
expectError: false,
featureEnabled: false,
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.IdentifyPodOS, test.featureEnabled)()
errs := validateWindows(test.sc, field.NewPath("field"))
if test.expectError && len(errs) > 0 {
if errs[0].Type != test.errorType {
@ -17847,36 +17742,20 @@ func TestValidateLinuxSecurityContext(t *testing.T) {
expectErr bool
errorType field.ErrorType
errorDetail string
featureEnabled bool
}{
"valid SC, linux, no error": {
sc: &core.PodSpec{Containers: []core.Container{{SecurityContext: validLinuxSC}}},
expectErr: false,
featureEnabled: true,
},
"invalid SC, linux, error": {
sc: &core.PodSpec{Containers: []core.Container{{SecurityContext: invalidLinuxSC}}},
errorType: "FieldValueForbidden",
errorDetail: "windows options cannot be set for a linux pod",
expectErr: true,
featureEnabled: true,
},
"valid SC, linux, no error, no IdentifyPodOS featuregate": {
sc: &core.PodSpec{Containers: []core.Container{{SecurityContext: validLinuxSC}}},
expectErr: false,
featureEnabled: false,
},
"invalid SC, linux, error, no IdentifyPodOS featuregate": {
sc: &core.PodSpec{Containers: []core.Container{{SecurityContext: invalidLinuxSC}}},
errorType: "FieldValueForbidden",
errorDetail: "windows options cannot be set for a linux pod",
expectErr: true,
featureEnabled: false,
},
}
for k, v := range cases {
t.Run(k, func(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.IdentifyPodOS, v.featureEnabled)()
errs := validateLinux(v.sc, field.NewPath("field"))
if v.expectErr && len(errs) > 0 {
if errs[0].Type != v.errorType || !strings.Contains(errs[0].Detail, v.errorDetail) {
@ -20379,81 +20258,42 @@ func TestValidateOS(t *testing.T) {
testCases := []struct {
name string
expectError bool
featureEnabled bool
podSpec *core.PodSpec
}{
{
name: "no OS field, featuregate",
expectError: false,
featureEnabled: true,
podSpec: &core.PodSpec{OS: nil},
},
{
name: "empty OS field, featuregate",
expectError: true,
featureEnabled: true,
podSpec: &core.PodSpec{OS: &core.PodOS{}},
},
{
name: "no OS field, no featuregate",
expectError: false,
featureEnabled: false,
podSpec: &core.PodSpec{OS: nil},
},
{
name: "empty OS field, no featuregate",
expectError: true,
featureEnabled: false,
podSpec: &core.PodSpec{OS: &core.PodOS{}},
},
{
name: "OS field, featuregate, valid OS",
expectError: false,
featureEnabled: true,
podSpec: &core.PodSpec{OS: &core.PodOS{Name: core.Linux}},
},
{
name: "OS field, featuregate, valid OS",
expectError: false,
featureEnabled: true,
podSpec: &core.PodSpec{OS: &core.PodOS{Name: core.Windows}},
},
{
name: "OS field, featuregate, empty OS",
expectError: true,
featureEnabled: true,
podSpec: &core.PodSpec{OS: &core.PodOS{Name: ""}},
},
{
name: "OS field, no featuregate, empty OS",
expectError: true,
featureEnabled: false,
podSpec: &core.PodSpec{OS: &core.PodOS{Name: ""}},
},
{
name: "OS field, featuregate, invalid OS",
expectError: true,
featureEnabled: true,
podSpec: &core.PodSpec{OS: &core.PodOS{Name: "dummyOS"}},
},
{
name: "OS field, no featuregate, valid OS",
expectError: true,
featureEnabled: false,
podSpec: &core.PodSpec{OS: &core.PodOS{Name: core.Linux}},
},
{
name: "OS field, no featuregate, invalid OS",
expectError: true,
featureEnabled: false,
podSpec: &core.PodSpec{OS: &core.PodOS{Name: "dummyOS"}},
},
}
for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.IdentifyPodOS, testCase.featureEnabled)()
errs := validateOS(testCase.podSpec, field.NewPath("spec"), PodValidationOptions{AllowOSField: testCase.featureEnabled})
errs := validateOS(testCase.podSpec, field.NewPath("spec"), PodValidationOptions{})
if testCase.expectError && len(errs) == 0 {
t.Errorf("Unexpected success")
}

3
pkg/features/kube_features.go
View File

@ -374,6 +374,7 @@ const (
// owner: @ravig
// alpha: v1.23
// beta: v1.24
// GA: v1.25
// IdentifyPodOS allows user to specify OS on which they'd like the Pod run. The user should still set the nodeSelector
// with appropriate `kubernetes.io/os` label for scheduler to identify appropriate node for the pod to run.
IdentifyPodOS featuregate.Feature = "IdentifyPodOS"
@ -904,7 +905,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
HonorPVReclaimPolicy: {Default: false, PreRelease: featuregate.Alpha},
IdentifyPodOS: {Default: true, PreRelease: featuregate.Beta},
IdentifyPodOS: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.27
InTreePluginAWSUnregister: {Default: false, PreRelease: featuregate.Alpha},