Skip to content

Commit 3790ee2

Browse files
SergeyKanzhelevSergeyKanzhelev
committed
reset fields when the feature gate was not set
1 parent 2253b53 commit 3790ee2

File tree

3 files changed

+312
-0
lines changed

3 files changed

+312
-0
lines changed

‎pkg/api/pod/util.go‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -813,6 +813,17 @@ func dropDisabledPodStatusFields(podStatus, oldPodStatus *api.PodStatus, podSpec
813813
}
814814
}
815815

816+
if !utilfeature.DefaultFeatureGate.Enabled(features.ResourceHealthStatus) {
817+
setAllocatedResourcesStatusToNil := func(csl []api.ContainerStatus) {
818+
for i := range csl {
819+
csl[i].AllocatedResourcesStatus = nil
820+
}
821+
}
822+
setAllocatedResourcesStatusToNil(podStatus.ContainerStatuses)
823+
setAllocatedResourcesStatusToNil(podStatus.InitContainerStatuses)
824+
setAllocatedResourcesStatusToNil(podStatus.EphemeralContainerStatuses)
825+
}
826+
816827
// drop ContainerStatus.User field to empty (disable SupplementalGroupsPolicy)
817828
if !utilfeature.DefaultFeatureGate.Enabled(features.SupplementalGroupsPolicy) && !supplementalGroupsPolicyInUse(oldPodSpec) {
818829
dropUserField := func(csl []api.ContainerStatus) {
@@ -1161,6 +1172,22 @@ func rroInUse(podSpec *api.PodSpec) bool {
11611172
return inUse
11621173
}
11631174

1175+
func allocatedResourcesStatusInUse(podSpec *api.PodStatus) bool {
1176+
if podSpec == nil {
1177+
return false
1178+
}
1179+
inUse := func(csl []api.ContainerStatus) bool {
1180+
for _, cs := range csl {
1181+
if len(cs.AllocatedResourcesStatus) > 0 {
1182+
return true
1183+
}
1184+
}
1185+
return false
1186+
}
1187+
1188+
return inUse(podSpec.ContainerStatuses) || inUse(podSpec.InitContainerStatuses) || inUse(podSpec.EphemeralContainerStatuses)
1189+
}
1190+
11641191
func dropDisabledClusterTrustBundleProjection(podSpec, oldPodSpec *api.PodSpec) {
11651192
if utilfeature.DefaultFeatureGate.Enabled(features.ClusterTrustBundleProjection) {
11661193
return

‎pkg/apis/core/validation/validation.go‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5378,6 +5378,11 @@ func ValidatePodStatusUpdate(newPod, oldPod *core.Pod, opts PodValidationOptions
53785378
allErrs = append(allErrs, validateContainerStatusUsers(newPod.Status.InitContainerStatuses, fldPath.Child("initContainerStatuses"), newPod.Spec.OS)...)
53795379
allErrs = append(allErrs, validateContainerStatusUsers(newPod.Status.EphemeralContainerStatuses, fldPath.Child("ephemeralContainerStatuses"), newPod.Spec.OS)...)
53805380

5381+
allErrs = append(allErrs, validateContainerStatusAllocatedResourcesStatus(newPod.Status.ContainerStatuses, fldPath.Child("containerStatuses"), newPod.Spec.Containers)...)
5382+
allErrs = append(allErrs, validateContainerStatusAllocatedResourcesStatus(newPod.Status.InitContainerStatuses, fldPath.Child("initContainerStatuses"), newPod.Spec.InitContainers)...)
5383+
// ephemeral containers are not allowed to have resources allocated
5384+
allErrs = append(allErrs, validateContainerStatusNoAllocatedResourcesStatus(newPod.Status.EphemeralContainerStatuses, fldPath.Child("ephemeralContainerStatuses"))...)
5385+
53815386
return allErrs
53825387
}
53835388

@@ -8200,6 +8205,80 @@ func validateContainerStatusUsers(containerStatuses []core.ContainerStatus, fldP
82008205
return allErrors
82018206
}
82028207

8208+
func validateContainerStatusNoAllocatedResourcesStatus(containerStatuses []core.ContainerStatus, fldPath *field.Path) field.ErrorList {
8209+
allErrors := field.ErrorList{}
8210+
8211+
for i, containerStatus := range containerStatuses {
8212+
if containerStatus.AllocatedResourcesStatus == nil {
8213+
continue
8214+
} else {
8215+
allErrors = append(allErrors, field.Forbidden(fldPath.Index(i).Child("allocatedResourcesStatus"), "cannot be set for a container status"))
8216+
}
8217+
}
8218+
8219+
return allErrors
8220+
}
8221+
8222+
// validateContainerStatusAllocatedResourcesStatus iterate the allocated resources health and validate:
8223+
// - resourceName matches one of resources in container's resource requirements
8224+
// - resourceID is not empty and unique
8225+
func validateContainerStatusAllocatedResourcesStatus(containerStatuses []core.ContainerStatus, fldPath *field.Path, containers []core.Container) field.ErrorList {
8226+
allErrors := field.ErrorList{}
8227+
8228+
for i, containerStatus := range containerStatuses {
8229+
if containerStatus.AllocatedResourcesStatus == nil {
8230+
continue
8231+
}
8232+
8233+
allocatedResources := containerStatus.AllocatedResourcesStatus
8234+
for j, allocatedResource := range allocatedResources {
8235+
var container core.Container
8236+
containerFound := false
8237+
// get container by name
8238+
for _, c := range containers {
8239+
if c.Name == containerStatus.Name {
8240+
containerFound = true
8241+
container = c
8242+
break
8243+
}
8244+
}
8245+
8246+
// ignore missing container, see https://github.com/kubernetes/kubernetes/issues/124915
8247+
if containerFound {
8248+
found := false
8249+
8250+
// get container resources from the spec
8251+
containerResources := container.Resources
8252+
for resourceName := range containerResources.Requests {
8253+
if resourceName == allocatedResource.Name {
8254+
found = true
8255+
break
8256+
}
8257+
}
8258+
if !found {
8259+
allErrors = append(allErrors, field.Invalid(fldPath.Index(i).Child("allocatedResourcesStatus").Index(j).Child("name"), allocatedResource.Name, "must match one of the container's resource requirements"))
8260+
}
8261+
}
8262+
8263+
uniqueResources := sets.New[core.ResourceID]()
8264+
// check resource IDs are unique
8265+
for k, r := range allocatedResource.Resources {
8266+
if r.Health != core.ResourceHealthStatusHealthy && r.Health != core.ResourceHealthStatusUnhealthy && r.Health != core.ResourceHealthStatusUnknown {
8267+
allErrors = append(allErrors, field.Invalid(fldPath.Index(i).Child("allocatedResourcesStatus").Index(j).Child("resources").Index(k).Child("health"), r.Health, "must be one of Healthy, Unhealthy, Unknown"))
8268+
}
8269+
8270+
if uniqueResources.Has(r.ResourceID) {
8271+
allErrors = append(allErrors, field.Invalid(fldPath.Index(i).Child("allocatedResourcesStatus").Index(j).Child("resources").Index(k).Child("resourceID"), r.ResourceID, "must be unique"))
8272+
} else {
8273+
uniqueResources.Insert(r.ResourceID)
8274+
}
8275+
}
8276+
}
8277+
}
8278+
8279+
return allErrors
8280+
}
8281+
82038282
func validateLinuxContainerUser(linuxContainerUser *core.LinuxContainerUser, fldPath *field.Path) field.ErrorList {
82048283
allErrors := field.ErrorList{}
82058284
if linuxContainerUser == nil {

‎pkg/apis/core/validation/validation_test.go‎

Lines changed: 206 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24423,3 +24423,209 @@ func TestValidatePodStatusUpdateWithSupplementalGroupsPolicy(t *testing.T) {
2442324423
}
2442424424
}
2442524425
}
24426+
func TestValidateContainerStatusNoAllocatedResourcesStatus(t *testing.T) {
24427+
containerStatuses := []core.ContainerStatus{
24428+
{
24429+
Name: "container-1",
24430+
},
24431+
{
24432+
Name: "container-2",
24433+
AllocatedResourcesStatus: []core.ResourceStatus{
24434+
{
24435+
Name: "test.device/test",
24436+
Resources: nil,
24437+
},
24438+
},
24439+
},
24440+
{
24441+
Name: "container-3",
24442+
AllocatedResourcesStatus: []core.ResourceStatus{
24443+
{
24444+
Name: "test.device/test",
24445+
Resources: []core.ResourceHealth{
24446+
{
24447+
ResourceID: "resource-1",
24448+
Health: core.ResourceHealthStatusHealthy,
24449+
},
24450+
},
24451+
},
24452+
},
24453+
},
24454+
}
24455+
24456+
fldPath := field.NewPath("spec", "containers")
24457+
24458+
errs := validateContainerStatusNoAllocatedResourcesStatus(containerStatuses, fldPath)
24459+
24460+
assert.Equal(t, 2, len(errs))
24461+
assert.Equal(t, "spec.containers[1].allocatedResourcesStatus", errs[0].Field)
24462+
assert.Equal(t, "cannot be set for a container status", errs[0].Detail)
24463+
assert.Equal(t, "spec.containers[2].allocatedResourcesStatus", errs[1].Field)
24464+
assert.Equal(t, "cannot be set for a container status", errs[1].Detail)
24465+
}
24466+
24467+
func TestValidateContainerStatusAllocatedResourcesStatus(t *testing.T) {
24468+
fldPath := field.NewPath("spec", "containers")
24469+
24470+
testCases := map[string]struct {
24471+
containers []core.Container
24472+
containerStatuses []core.ContainerStatus
24473+
wantFieldErrors field.ErrorList
24474+
}{
24475+
"basic correct status": {
24476+
containers: []core.Container{
24477+
{
24478+
Name: "container-1",
24479+
Resources: core.ResourceRequirements{
24480+
Requests: core.ResourceList{
24481+
"test.device/test": resource.MustParse("1"),
24482+
},
24483+
},
24484+
},
24485+
},
24486+
containerStatuses: []core.ContainerStatus{
24487+
{
24488+
Name: "container-1",
24489+
AllocatedResourcesStatus: []core.ResourceStatus{
24490+
{
24491+
Name: "test.device/test",
24492+
Resources: []core.ResourceHealth{
24493+
{
24494+
ResourceID: "resource-1",
24495+
Health: core.ResourceHealthStatusHealthy,
24496+
},
24497+
},
24498+
},
24499+
},
24500+
},
24501+
},
24502+
wantFieldErrors: field.ErrorList{},
24503+
},
24504+
"ignoring the missing container (see https://github.com/kubernetes/kubernetes/issues/124915)": {
24505+
containers: []core.Container{
24506+
{
24507+
Name: "container-2",
24508+
Resources: core.ResourceRequirements{
24509+
Requests: core.ResourceList{
24510+
"test.device/test": resource.MustParse("1"),
24511+
},
24512+
},
24513+
},
24514+
},
24515+
containerStatuses: []core.ContainerStatus{
24516+
{
24517+
Name: "container-1",
24518+
AllocatedResourcesStatus: []core.ResourceStatus{
24519+
{
24520+
Name: "test.device/test",
24521+
Resources: []core.ResourceHealth{
24522+
{
24523+
ResourceID: "resource-1",
24524+
Health: core.ResourceHealthStatusHealthy,
24525+
},
24526+
},
24527+
},
24528+
},
24529+
},
24530+
},
24531+
wantFieldErrors: field.ErrorList{},
24532+
},
24533+
"allow nil": {
24534+
containers: []core.Container{
24535+
{
24536+
Name: "container-2",
24537+
Resources: core.ResourceRequirements{
24538+
Requests: core.ResourceList{
24539+
"test.device/test": resource.MustParse("1"),
24540+
},
24541+
},
24542+
},
24543+
},
24544+
containerStatuses: []core.ContainerStatus{
24545+
{
24546+
Name: "container-1",
24547+
},
24548+
},
24549+
wantFieldErrors: field.ErrorList{},
24550+
},
24551+
"don't allow non-unique IDs": {
24552+
containers: []core.Container{
24553+
{
24554+
Name: "container-2",
24555+
Resources: core.ResourceRequirements{
24556+
Requests: core.ResourceList{
24557+
"test.device/test": resource.MustParse("1"),
24558+
},
24559+
},
24560+
},
24561+
},
24562+
containerStatuses: []core.ContainerStatus{
24563+
{
24564+
Name: "container-1",
24565+
AllocatedResourcesStatus: []core.ResourceStatus{
24566+
{
24567+
Name: "test.device/test",
24568+
Resources: []core.ResourceHealth{
24569+
{
24570+
ResourceID: "resource-1",
24571+
Health: core.ResourceHealthStatusHealthy,
24572+
},
24573+
{
24574+
ResourceID: "resource-1",
24575+
Health: core.ResourceHealthStatusUnhealthy,
24576+
},
24577+
},
24578+
},
24579+
},
24580+
},
24581+
},
24582+
wantFieldErrors: field.ErrorList{
24583+
field.Invalid(fldPath.Index(0).Child("allocatedResourcesStatus").Index(0).Child("resources").Index(1).Child("resourceID"), core.ResourceID("resource-1"), "must be unique"),
24584+
},
24585+
},
24586+
24587+
"don't allow resources that are not in spec": {
24588+
containers: []core.Container{
24589+
{
24590+
Name: "container-1",
24591+
Resources: core.ResourceRequirements{
24592+
Requests: core.ResourceList{
24593+
"test.device/test": resource.MustParse("1"),
24594+
},
24595+
},
24596+
},
24597+
},
24598+
containerStatuses: []core.ContainerStatus{
24599+
{
24600+
Name: "container-1",
24601+
AllocatedResourcesStatus: []core.ResourceStatus{
24602+
{
24603+
Name: "test.device/test",
24604+
Resources: []core.ResourceHealth{
24605+
{
24606+
ResourceID: "resource-1",
24607+
Health: core.ResourceHealthStatusHealthy,
24608+
},
24609+
},
24610+
},
24611+
{
24612+
Name: "test.device/test2",
24613+
Resources: []core.ResourceHealth{},
24614+
},
24615+
},
24616+
},
24617+
},
24618+
wantFieldErrors: field.ErrorList{
24619+
field.Invalid(fldPath.Index(0).Child("allocatedResourcesStatus").Index(1).Child("name"), core.ResourceName("test.device/test2"), "must match one of the container's resource requirements"),
24620+
},
24621+
},
24622+
}
24623+
for name, tt := range testCases {
24624+
t.Run(name, func(t *testing.T) {
24625+
errs := validateContainerStatusAllocatedResourcesStatus(tt.containerStatuses, fldPath, tt.containers)
24626+
if diff := cmp.Diff(tt.wantFieldErrors, errs); diff != "" {
24627+
t.Errorf("unexpected field errors (-want, +got):\n%s", diff)
24628+
}
24629+
})
24630+
}
24631+
}

0 commit comments

Comments
 (0)