Fix local isolation for pod requesting only overlay or scratch

[ddysher]

What this PR does / why we need it:

Fix overlay resource predicates for pod with only overlay or scratch storage request.

E.g. the following pod can pass predicate even if overlay is only 512Gi.

apiVersion: v1
kind: Pod
metadata:
  name: pod
spec:
  containers:
  - name: nginx
    image: nginx
    resources:
      requests:
        storage.kubernetes.io/overlay: 1024Gi

similarly, following pod will also pass predicate

apiVersion: v1
kind: Pod
metadata:
  name: pod
spec:
  containers:
  - name: nginx
    image: nginx
    volumeMounts:
    - name: data
      mountPath: /data
  volumes:
  - name: data
    emptyDir:
      sizeLimit: 1024Gi

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #47798

Special notes for your reviewer:

Release note:

@jingxu97 @vishh @dashpole

[k8s-ci-robot]

Hi @ddysher. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with @k8s-bot ok to test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

[ddysher]

I copied this from other test cases, but do not understand why v1.ResourceStorageScratch is used in test instead of v1.ResourceStorageOverlay?

[k82cn]

@kubernetes/sig-scheduling-pr-reviews , anyone have related knowledge for v1.ResourceStorageScratch?

[jingxu97]

v1.ResourceStorageScratch is added in v1.7 to represent the capacity of local storage for scratch space (e.g., used for emptyDir, and also overlay and imagefs if no separate imagefs setup). So ResourceStorageScratch represents the capacity, and ResourceStorageOverlay represents one type of request

[jingxu97]

/approve

[jingxu97]

/lgtm.
Thank you @ddysher for your fix!

[jingxu97]

@davidopp Could you please help approve this PR? Thanks!

[k82cn]

@k8s-bot ok to test

[vishh]

/lgtm

Thanks for the fix @ddysher

[k82cn]

/release-note-none

[k82cn]

hm.. let me check StorageOverlay and review it :).

[ddysher]

@k82cn FYI #46456

@jingxu97 @vishh np! Thanks for putting this together; this is really a long wanted feature :-)

[k82cn]

lgtm; this PR add a check for storage request, only return true if all (cpu, memory, storage, opq) request resources are zero.

But there's some comments about the previous PR (#46456):

1. In Reosurce.ResourceList(), it do not include StorageScratch; no UT broken, but it's better to include all resources here.
2. In the storage test, it's better to also add Node's allocation, similar to the other test; I had to go through the for-loop to know how many storage here, and we can not set it case by case

I'll approve this PR and create another one for those two cleanup comments.

[k82cn]

/approve

[k82cn]

/approve no-issue

[ddysher]

@jingxu97 @vishh @k82cn I've updated the PR to include scratch as well.

[k82cn]

add a description for this case.

[ddysher]

done. it's just a one line change, so I force update the second commit.

[k82cn]

lgtm, just add a description of the new case :).

[ddysher]

description added, but it's not a new case though. git diff is misleading

lgtm, just add a description of the new case :).

[jingxu97]

/lgtm

[k82cn]

@vishh , @jingxu97 , can you help to remove do-not-merge label ?

[ddysher]

@vishh @jingxu97 Do we want to make it for v1.7?

[vishh]

/lgtm

[k8s-github-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ddysher, jingxu97, k82cn, vishh

Associated issue requirement bypassed by: k82cn

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

• plugin/pkg/scheduler/OWNERS [k82cn]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

[vishh]

@ddysher can you file an issue to get release approval?

[ddysher]

@vishh done

ref: #47798

[vishh]

cc @saad-ali @dchen1107

[saad-ali]

Fix is low risk, approved for 1.7

[marun]

CI seems to be stalled.

/test pull-kubernetes-federation-e2e-gce

[k8s-github-robot]

Automatic merge from submit-queue (batch tested with PRs 47883, 47179, 46966, 47982, 47945)