Introduce PodHasNetwork condition for pods #111358

ddebroy · 2022-07-23T00:30:14Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

This PR introduces a new PodHasNetwork pod condition managed by Kubelet and applied on pods. The condition is set when the pod sandbox has been created and the network configured by the CRI runtime. For use cases of how this condition will be consumed, please see KEP.

Example: Pod referencing a missing configmap (and does not specify init containers):

$ kubectl describe pod nginx5
Name:             nginx5
Namespace:        default
...
Conditions:
  Type              Status
  PodHasNetwork     False 
  Initialized       True 
  Ready             False 
  ContainersReady   False 
  PodScheduled      True
...
Events:
  Type     Reason       Age              From               Message
  ----     ------       ----             ----               -------
  Normal   Scheduled    5s               default-scheduler  Successfully assigned default/nginx5 to kubernetes-minion-group-ggrq
  Warning  FailedMount  1s (x4 over 5s)  kubelet            MountVolume.SetUp failed for volume "config-volume" : configmap "game-config" not found

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

As described in the KEP, the decisions around setting this condition is based on KubeRuntime's existing PodSandboxChanged function. This PR moves the PodSandboxChanged function and a couple of dependent functions to a fresh kubeRuntime/utils location so that the status manager component can import the logic without cyclical dependencies.

Please see notes about a API review not necessary at Alpha stage for this in #111358 (comment)

Does this PR introduce a user-facing change?

Introduce PodHasNetwork condition for pods

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

KEP-3085: Pod network ready condition

k8s-ci-robot · 2022-07-23T00:30:22Z

@ddebroy: This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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.

ddebroy · 2022-07-23T00:32:42Z

/sig-node

k8s-triage-robot · 2022-07-23T01:01:49Z

This PR may require API review.

If so, when the changes are ready, complete the pre-review checklist and request an API review.

Status of requested reviews is tracked in the API Review project.

ddebroy · 2022-07-23T03:58:47Z

/test pull-kubernetes-conformance-kind-ga-only-parallel

ddebroy · 2022-07-23T06:00:31Z

/label api-review

ddebroy · 2022-07-25T07:03:55Z

/remove-label api-review

ddebroy · 2022-07-25T07:08:17Z

pkg/kubelet/types/constants.go

@@ -43,3 +43,13 @@ const (
 	LimitedSwap   = "LimitedSwap"
 	UnlimitedSwap = "UnlimitedSwap"
 )
+
+// Alpha conditions managed by Kubelet that are not yet part of the API. The


This follows the advice from @liggitt around constants associated with new conditions at #110959 (comment)

Also confirmed at https://kubernetes.slack.com/archives/C0BP8PW9G/p1658765248399249?thread_ts=1658759334.709079&cid=C0BP8PW9G

this confused me. so thanks for the pointer.

ddebroy · 2022-07-25T07:10:51Z

/assign @derekwaynecarr

Signed-off-by: Deep Debroy <ddebroy@gmail.com>

ddebroy · 2022-08-01T17:05:15Z

/test pull-node-e2e-serial containerd

k8s-ci-robot · 2022-08-01T17:05:17Z

@ddebroy: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test pull-kubernetes-conformance-kind-ga-only-parallel
/test pull-kubernetes-coverage-unit
/test pull-kubernetes-dependencies
/test pull-kubernetes-dependencies-go-canary
/test pull-kubernetes-e2e-gce
/test pull-kubernetes-e2e-gce-100-performance
/test pull-kubernetes-e2e-gce-big-performance
/test pull-kubernetes-e2e-gce-canary
/test pull-kubernetes-e2e-gce-network-proxy-http-connect
/test pull-kubernetes-e2e-gce-no-stage
/test pull-kubernetes-e2e-gce-scale-performance-manual
/test pull-kubernetes-e2e-gce-ubuntu-containerd
/test pull-kubernetes-e2e-gce-ubuntu-containerd-canary
/test pull-kubernetes-e2e-kind
/test pull-kubernetes-e2e-kind-ipv6
/test pull-kubernetes-files-remake
/test pull-kubernetes-integration
/test pull-kubernetes-integration-go-canary
/test pull-kubernetes-kubemark-e2e-gce-scale
/test pull-kubernetes-node-e2e-containerd
/test pull-kubernetes-typecheck
/test pull-kubernetes-unit
/test pull-kubernetes-unit-go-canary
/test pull-kubernetes-update
/test pull-kubernetes-verify
/test pull-kubernetes-verify-go-canary
/test pull-kubernetes-verify-govet-levee

The following commands are available to trigger optional jobs:

/test check-dependency-stats
/test pull-ci-kubernetes-unit-windows
/test pull-kubernetes-conformance-image-test
/test pull-kubernetes-conformance-kind-ga-only
/test pull-kubernetes-conformance-kind-ipv6-parallel
/test pull-kubernetes-cross
/test pull-kubernetes-e2e-capz-azure-disk
/test pull-kubernetes-e2e-capz-azure-disk-vmss
/test pull-kubernetes-e2e-capz-azure-file
/test pull-kubernetes-e2e-capz-azure-file-vmss
/test pull-kubernetes-e2e-capz-conformance
/test pull-kubernetes-e2e-capz-ha-control-plane
/test pull-kubernetes-e2e-capz-windows-containerd
/test pull-kubernetes-e2e-containerd-gce
/test pull-kubernetes-e2e-gce-alpha-features
/test pull-kubernetes-e2e-gce-correctness
/test pull-kubernetes-e2e-gce-csi-serial
/test pull-kubernetes-e2e-gce-device-plugin-gpu
/test pull-kubernetes-e2e-gce-kubetest2
/test pull-kubernetes-e2e-gce-network-proxy-grpc
/test pull-kubernetes-e2e-gce-storage-disruptive
/test pull-kubernetes-e2e-gce-storage-slow
/test pull-kubernetes-e2e-gce-storage-snapshot
/test pull-kubernetes-e2e-gce-ubuntu-containerd-serial
/test pull-kubernetes-e2e-gci-gce-autoscaling
/test pull-kubernetes-e2e-gci-gce-ingress
/test pull-kubernetes-e2e-gci-gce-ipvs
/test pull-kubernetes-e2e-kind-canary
/test pull-kubernetes-e2e-kind-dual-canary
/test pull-kubernetes-e2e-kind-ipv6-canary
/test pull-kubernetes-e2e-kind-ipvs-dual-canary
/test pull-kubernetes-e2e-kind-multizone
/test pull-kubernetes-e2e-kops-aws
/test pull-kubernetes-e2e-ubuntu-gce-network-policies
/test pull-kubernetes-kubemark-e2e-gce-big
/test pull-kubernetes-local-e2e
/test pull-kubernetes-node-crio-cgrpv2-e2e
/test pull-kubernetes-node-crio-cgrpv2-e2e-kubetest2
/test pull-kubernetes-node-crio-e2e
/test pull-kubernetes-node-crio-e2e-kubetest2
/test pull-kubernetes-node-e2e-containerd-alpha-features
/test pull-kubernetes-node-e2e-containerd-features
/test pull-kubernetes-node-e2e-containerd-features-kubetest2
/test pull-kubernetes-node-e2e-containerd-kubetest2
/test pull-kubernetes-node-kubelet-credential-provider
/test pull-kubernetes-node-kubelet-serial-containerd
/test pull-kubernetes-node-kubelet-serial-containerd-kubetest2
/test pull-kubernetes-node-kubelet-serial-cpu-manager
/test pull-kubernetes-node-kubelet-serial-cpu-manager-kubetest2
/test pull-kubernetes-node-kubelet-serial-crio-cgroupv1
/test pull-kubernetes-node-kubelet-serial-crio-cgroupv2
/test pull-kubernetes-node-kubelet-serial-hugepages
/test pull-kubernetes-node-kubelet-serial-memory-manager
/test pull-kubernetes-node-kubelet-serial-topology-manager
/test pull-kubernetes-node-kubelet-serial-topology-manager-kubetest2
/test pull-kubernetes-node-memoryqos-cgrpv2
/test pull-kubernetes-node-swap-fedora
/test pull-kubernetes-node-swap-fedora-serial
/test pull-kubernetes-node-swap-ubuntu-serial
/test pull-kubernetes-unit-experimental
/test pull-publishing-bot-validate

Use /test all to run the following jobs that were automatically triggered:

pull-kubernetes-conformance-kind-ga-only-parallel
pull-kubernetes-conformance-kind-ipv6-parallel
pull-kubernetes-dependencies
pull-kubernetes-e2e-gce-100-performance
pull-kubernetes-e2e-gce-alpha-features
pull-kubernetes-e2e-gce-ubuntu-containerd
pull-kubernetes-e2e-kind
pull-kubernetes-e2e-kind-ipv6
pull-kubernetes-integration
pull-kubernetes-node-e2e-containerd
pull-kubernetes-typecheck
pull-kubernetes-unit
pull-kubernetes-verify
pull-kubernetes-verify-govet-levee

In response to this:

/test pull-node-e2e-serial containerd

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.

ddebroy · 2022-08-01T17:06:13Z

/test pull-kubernetes-node-kubelet-serial-containerd

derekwaynecarr

The kubelet changes all look great, and I think the condition will help users!

I had expected to see the kube-apiserver drop usage of the alpha condition in DropDisabledFields for the associated pod so clients would not see the condition when its not enabled on kube-apiserver feature gate.

@liggitt can you confirm you share the same expectation? by keeping conditions during the alpha phase as a const local to the writer (kubelet), we lack the mirror capability of dropping the field on kube-apiserver side. is there a write-up about conditions that says dropping is not required that I missed?

derekwaynecarr · 2022-08-01T17:20:32Z

pkg/kubelet/types/constants.go

@@ -43,3 +43,13 @@ const (
 	LimitedSwap   = "LimitedSwap"
 	UnlimitedSwap = "UnlimitedSwap"
 )
+
+// Alpha conditions managed by Kubelet that are not yet part of the API. The


this confused me. so thanks for the pointer.

ddebroy · 2022-08-01T17:42:42Z

/test pull-kubernetes-e2e-kind-ipv6

liggitt · 2022-08-01T18:05:29Z

I had expected to see the kube-apiserver drop usage of the alpha condition in DropDisabledFields for the associated pod so clients would not see the condition when its not enabled on kube-apiserver feature gate.

Dropping disabled fields is done for new API fields so we don't allow persisting data in fields an n-1 API server doesn't know about and can't preserve. If this feature is using pod conditions, the fields are not new, just the particular values in the fields, right? I wouldn't expect the API to drop specific pod conditions based on feature gates.

derekwaynecarr · 2022-08-01T18:30:47Z

@liggitt thanks for the reply.

/approve
/lgtm

derekwaynecarr · 2022-08-01T18:31:15Z

/milestone v1.25

k8s-ci-robot · 2022-08-01T18:31:23Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ddebroy, derekwaynecarr

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~pkg/features/OWNERS~~ [derekwaynecarr]
~~pkg/kubelet/OWNERS~~ [derekwaynecarr]
~~test/e2e_node/OWNERS~~ [derekwaynecarr]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ddebroy · 2022-08-01T18:45:58Z

/test pull-kubernetes-node-kubelet-serial-containerd

ddebroy · 2022-08-01T18:46:40Z

/hold
confirming pull-kubernetes-node-kubelet-serial-containerd results

derekwaynecarr · 2022-08-01T18:58:52Z

/lgtm

k8s-ci-robot · 2022-08-01T20:03:07Z

@ddebroy: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-kubernetes-node-kubelet-serial-containerd	`0ac7cce`	link	false	`/test pull-kubernetes-node-kubelet-serial-containerd`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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.

ddebroy · 2022-08-01T20:07:25Z

Validated the logs in pull-kubernetes-node-kubelet-serial-containerd and it appears the new tests with [sig-node] Pod conditions managed by Kubelet including PodHasNetwork condition [Serial] [Feature:PodHasNetwork] are passing. There is some other unrelated infra issue that is causing the failure.

ddebroy · 2022-08-01T20:08:33Z

/remove-hold

aojea · 2022-09-05T21:13:36Z

to follow up, what will be the expectation for hostNetwork pods?

ddebroy · 2022-09-06T17:23:40Z

to follow up, what will be the expectation for hostNetwork pods?

@aojea, for pods with hostNetwork configured, the fields in sandboxStatus.Network are not considered when setting the PodHasNetwork condition to true. Only the overall CRI sandboxStatus is considered. Thus, as soon as CRI sandbox is successfully created for a pod with host network, the PodHasNetwork condition will be set. I will add this clarification to the docs for this condition.

aojea · 2022-09-08T09:18:25Z

thanks for the prompt response

k8s-ci-robot added needs-priority Indicates a PR lacks a `priority/foo` label and requires one. area/kubelet labels Jul 23, 2022

k8s-ci-robot requested review from dchen1107 and derekwaynecarr July 23, 2022 00:30

ddebroy force-pushed the hasnet1 branch from 65eaab0 to fefc874 Compare July 23, 2022 01:45

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 23, 2022

ddebroy force-pushed the hasnet1 branch from fefc874 to 56c1ccc Compare July 23, 2022 04:03

k8s-ci-robot added the api-review Categorizes an issue or PR as actively needing an API review. label Jul 23, 2022

Priyankasaggu11929 mentioned this pull request Jul 25, 2022

Pod conditions around readiness to start containers after completion of pod sandbox creation kubernetes/enhancements#3085

Closed

10 tasks

ddebroy force-pushed the hasnet1 branch from 56c1ccc to 2eda386 Compare July 25, 2022 07:01

k8s-ci-robot removed the api-review Categorizes an issue or PR as actively needing an API review. label Jul 25, 2022

ddebroy commented Jul 25, 2022

View reviewed changes

Node e2e test for pod conditions managed by Kubelet

0ac7cce

Signed-off-by: Deep Debroy <ddebroy@gmail.com>

ddebroy force-pushed the hasnet1 branch from 626aaf1 to 0ac7cce Compare August 1, 2022 16:52

derekwaynecarr requested changes Aug 1, 2022

View reviewed changes

k8s-ci-robot added this to the v1.25 milestone Aug 1, 2022

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 1, 2022

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 1, 2022

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 1, 2022

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 1, 2022

k8s-ci-robot merged commit 2e1a4da into kubernetes:master Aug 1, 2022

SIG Node PR Triage automation moved this from Triage to Done Aug 1, 2022

ddebroy mentioned this pull request Sep 6, 2022

Add blog post for PodHasNetwork condition kubernetes/website#36197

Merged

ddebroy mentioned this pull request Sep 9, 2022

Clarify hostNetwork and address other outstanding feedback kubernetes/website#36710

Merged

kannon92 mentioned this pull request Jul 26, 2023

[KEP-3085] Add condition for sandbox creation (xposted from original issue) kubernetes/enhancements#4138

Open

20 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce PodHasNetwork condition for pods #111358

Introduce PodHasNetwork condition for pods #111358

ddebroy commented Jul 23, 2022 •

edited

k8s-ci-robot commented Jul 23, 2022

ddebroy commented Jul 23, 2022

k8s-triage-robot commented Jul 23, 2022

ddebroy commented Jul 23, 2022

ddebroy commented Jul 23, 2022

ddebroy commented Jul 25, 2022

ddebroy Jul 25, 2022

ddebroy Jul 25, 2022

derekwaynecarr Aug 1, 2022

ddebroy commented Jul 25, 2022

ddebroy commented Aug 1, 2022

k8s-ci-robot commented Aug 1, 2022

ddebroy commented Aug 1, 2022

derekwaynecarr left a comment

derekwaynecarr Aug 1, 2022

ddebroy commented Aug 1, 2022

liggitt commented Aug 1, 2022

derekwaynecarr commented Aug 1, 2022 •

edited

derekwaynecarr commented Aug 1, 2022

k8s-ci-robot commented Aug 1, 2022

ddebroy commented Aug 1, 2022

ddebroy commented Aug 1, 2022

derekwaynecarr commented Aug 1, 2022

k8s-ci-robot commented Aug 1, 2022

ddebroy commented Aug 1, 2022 •

edited

ddebroy commented Aug 1, 2022

aojea commented Sep 5, 2022

ddebroy commented Sep 6, 2022 •

edited

aojea commented Sep 8, 2022

Introduce PodHasNetwork condition for pods #111358

Introduce PodHasNetwork condition for pods #111358

Conversation

ddebroy commented Jul 23, 2022 • edited

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

k8s-ci-robot commented Jul 23, 2022

ddebroy commented Jul 23, 2022

k8s-triage-robot commented Jul 23, 2022

ddebroy commented Jul 23, 2022

ddebroy commented Jul 23, 2022

ddebroy commented Jul 25, 2022

ddebroy Jul 25, 2022

Choose a reason for hiding this comment

ddebroy Jul 25, 2022

Choose a reason for hiding this comment

derekwaynecarr Aug 1, 2022

Choose a reason for hiding this comment

ddebroy commented Jul 25, 2022

ddebroy commented Aug 1, 2022

k8s-ci-robot commented Aug 1, 2022

ddebroy commented Aug 1, 2022

derekwaynecarr left a comment

Choose a reason for hiding this comment

derekwaynecarr Aug 1, 2022

Choose a reason for hiding this comment

ddebroy commented Aug 1, 2022

liggitt commented Aug 1, 2022

derekwaynecarr commented Aug 1, 2022 • edited

derekwaynecarr commented Aug 1, 2022

k8s-ci-robot commented Aug 1, 2022

ddebroy commented Aug 1, 2022

ddebroy commented Aug 1, 2022

derekwaynecarr commented Aug 1, 2022

k8s-ci-robot commented Aug 1, 2022

ddebroy commented Aug 1, 2022 • edited

ddebroy commented Aug 1, 2022

aojea commented Sep 5, 2022

ddebroy commented Sep 6, 2022 • edited

aojea commented Sep 8, 2022

ddebroy commented Jul 23, 2022 •

edited

derekwaynecarr commented Aug 1, 2022 •

edited

ddebroy commented Aug 1, 2022 •

edited

ddebroy commented Sep 6, 2022 •

edited