Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Updated Create Pod+PodStatus resource lifecycle test - +4 endpoint coverage #93459

Merged
merged 3 commits into from Nov 7, 2020
Merged

Updated Create Pod+PodStatus resource lifecycle test - +4 endpoint coverage #93459

merged 3 commits into from Nov 7, 2020

Conversation

riaankleinhans
Copy link
Contributor

What type of PR is this?
/kind cleanup

What this PR does / why we need it:
This PR adds a test to test the following untested endpoints:

  • replaceCoreV1NamespacedPodStatus
  • readCoreV1NamespacedPodStatus
  • listCoreV1PodForAllNamespaces
  • deleteCoreV1CollectionNamespacedPod

Which issue(s) this PR fixes:
Fixes #90936

Special notes for your reviewer:
Adds +4 endpoint test coverage (good for conformance)

Does this PR introduce a user-facing change?:

NONE

Release note:

NONE

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

NONE

/sig testing
/sig architecture
/area conformance

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. sig/testing Categorizes an issue or PR as relevant to SIG Testing. sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. area/conformance Issues or PRs related to kubernetes conformance tests cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. area/test labels Jul 27, 2020
@BobyMCbobs BobyMCbobs force-pushed the ii-create-pod%2Bpodstatus-resource-lifecycle-test branch from c8b7878 to e5225ab Compare July 27, 2020 02:14
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jul 27, 2020
@BobyMCbobs BobyMCbobs force-pushed the ii-create-pod%2Bpodstatus-resource-lifecycle-test branch from a3b168b to e08741c Compare July 27, 2020 04:25
dashpole
dashpole reviewed Jul 27, 2020
View reviewed changes
test/e2e/common/pods.go
podStatusUpdate, err := f.ClientSet.CoreV1().Pods(testNamespaceName).UpdateStatus(context.TODO(), &podStatusUpdated, metav1.UpdateOptions{})
framework.ExpectNoError(err, "failed to update PodStatus of Pod %s in namespace %s", testPodName, testNamespaceName)

ginkgo.By("check the Pod again to ensure its Ready conditions are False")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't there the possibility that the kubelet has already updated the pod's ready condition back to true here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might be wrong, but from my understand the response from the API server (in this case podStatusUpdate) is basically the same as the request. If that 's true then it should be fine to do this, otherwise I'll remove the checks after the update (so the endpoint is hit)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I misread the code, and thought we were doing a separate retrieval

@riaankleinhans riaankleinhans added this to In Progress PRs in conformance-definition Jul 27, 2020
@BobyMCbobs
Copy link
Member

/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 28, 2020
@BobyMCbobs
Copy link
Member

/retest

@BobyMCbobs
Copy link
Member

/test all

@BobyMCbobs
Copy link
Member

/retest

4 similar comments
@BobyMCbobs
Copy link
Member

/retest

@BobyMCbobs
Copy link
Member

/retest

@BobyMCbobs
Copy link
Member

/retest

@BobyMCbobs
Copy link
Member

/retest

@BobyMCbobs
Copy link
Member

/unhold

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 29, 2020
@heyste
Copy link
Member

heyste commented Nov 5, 2020

/test pull-kubernetes-e2e-gce-ubuntu-containerd
flake

[sig-auth] ServiceAccounts should guarantee kube-root-ca.crt exist in any namespace [It]
{"msg":"FAILED [sig-auth] ServiceAccounts should guarantee kube-root-ca.crt exist in any namespace","total":-1,"completed":19,"skipped":141,"failed":1,"failures":["[sig-auth] ServiceAccounts should guarantee kube-root-ca.crt exist in any namespace"]}

@liggitt
Copy link
Member

liggitt commented Nov 5, 2020 •
edited

fix for that flake in #96277

@heyste
Copy link
Member

heyste commented Nov 5, 2020

/test pull-kubernetes-e2e-gce-ubuntu-containerd
flakes

Kubernetes e2e suite: [sig-storage] CSI Volumes [Driver: csi-hostpath] [Testpattern: Dynamic PV (default fs)] subPath should support readOnly file specified in the volumeMount [LinuxOnly]
...
expected pod "pod-subpath-test-dynamicpv-r77v" success: Gave up after waiting 5m0s for pod "pod-subpath-test-dynamicpv-r77v" to be "Succeeded or Failed"

Kubernetes e2e suite: [sig-storage] In-tree Volumes [Driver: gluster] [Testpattern: Pre-provisioned PV (filesystem volmode)] volumeMode should not mount / map unused volumes in a pod
...
error checking directory /var/lib/kubelet/pods/ed5b351a-b2c6-44b4-b894-5a17ea54b5c4/volumeDevices on node e2e-c10f5eb34f-a7d53-minion-group-c0lk: error dialing backend: dial tcp 10.40.0.4:10250: connect: connection refused

Kubernetes e2e suite: [sig-network] Networking Granular Checks: Services should function for pod-Service: udp
...
Nov  5 23:09:46.105: failed dialing endpoint, did not find expected responses... 
Tries 39
Command curl -g -q -s 'http://10.64.1.234:8080/dial?request=hostname&protocol=udp&host=10.0.252.8&port=90&tries=1'
retrieved map[netserver-0:{} netserver-2:{}]
expected map[netserver-0:{} netserver-1:{} netserver-2:{}]
vendor/github.com/onsi/ginkgo/internal/leafnodes/runner.go:113

@heyste
Copy link
Member

heyste commented Nov 6, 2020

@spiffxp Added a constant as suggested, PodReadyTimeout and checks are looking good now.

@spiffxp
Copy link
Member

spiffxp commented Nov 6, 2020

/approve
needs rebase, ping me for lgtm after that

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 6, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dashpole, Riaankl, spiffxp

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:

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 6, 2020
@hh hh force-pushed the ii-create-pod%2Bpodstatus-resource-lifecycle-test branch from d8099c0 to 36f4527 Compare November 6, 2020 19:29
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 6, 2020
@hh
Copy link
Member

hh commented Nov 6, 2020

Rebased

spiffxp
spiffxp reviewed Nov 6, 2020
View reviewed changes
Copy link
Member

@spiffxp spiffxp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 6, 2020
@hh
Copy link
Member

hh commented Nov 6, 2020

/retest pull-kubernetes-e2e-gce-ubuntu-containerd

@k8s-ci-robot
Copy link
Contributor

@hh: The /retest command does not accept any targets.
The following commands are available to trigger jobs:

  • /test pull-kubernetes-bazel-build
  • /test pull-kubernetes-bazel-test
  • /test pull-kubernetes-conformance-image-test
  • /test pull-kubernetes-conformance-kind-ipv6-parallel
  • /test pull-kubernetes-dependencies
  • /test pull-kubernetes-dependencies-canary
  • /test pull-kubernetes-e2e-ipvs-azure-dualstack
  • /test pull-kubernetes-e2e-iptables-azure-dualstack
  • /test pull-kubernetes-e2e-aws-eks-1-13-correctness
  • /test pull-kubernetes-files-remake
  • /test pull-kubernetes-e2e-gce
  • /test pull-kubernetes-e2e-gce-no-stage
  • /test pull-kubernetes-e2e-gce-kubetest2
  • /test pull-kubernetes-e2e-gce-canary
  • /test pull-kubernetes-e2e-gce-ubuntu
  • /test pull-kubernetes-e2e-gce-ubuntu-containerd
  • /test pull-kubernetes-e2e-gce-ubuntu-containerd-canary
  • /test pull-kubernetes-e2e-gce-rbe
  • /test pull-kubernetes-e2e-gce-alpha-features
  • /test pull-kubernetes-e2e-gce-device-plugin-gpu
  • /test pull-kubernetes-integration
  • /test pull-kubernetes-cross
  • /test pull-kubernetes-e2e-kind
  • /test pull-kubernetes-e2e-kind-canary
  • /test pull-kubernetes-e2e-kind-ipv6
  • /test pull-kubernetes-e2e-kind-ipv6-canary
  • /test pull-kubernetes-conformance-kind-ga-only
  • /test pull-kubernetes-conformance-kind-ga-only-parallel
  • /test pull-kubernetes-e2e-kops-aws
  • /test pull-kubernetes-bazel-build-canary
  • /test pull-kubernetes-bazel-test-canary
  • /test pull-kubernetes-bazel-test-integration-canary
  • /test pull-kubernetes-local-e2e
  • /test pull-publishing-bot-validate
  • /test pull-kubernetes-e2e-gce-network-proxy-http-connect
  • /test pull-kubernetes-e2e-gce-network-proxy-grpc
  • /test pull-kubernetes-e2e-gci-gce-autoscaling
  • /test pull-kubernetes-e2e-aks-engine-azure
  • /test pull-kubernetes-e2e-azure-disk
  • /test pull-kubernetes-e2e-azure-disk-vmss
  • /test pull-kubernetes-e2e-azure-file
  • /test pull-kubernetes-e2e-kind-dual-canary
  • /test pull-kubernetes-e2e-kind-ipvs-dual-canary
  • /test pull-kubernetes-e2e-gci-gce-ipvs
  • /test pull-kubernetes-node-e2e
  • /test pull-kubernetes-e2e-containerd-gce
  • /test pull-kubernetes-node-e2e-containerd
  • /test pull-kubernetes-node-e2e-alpha
  • /test pull-kubernetes-node-kubelet-serial-cpu-manager
  • /test pull-kubernetes-node-kubelet-serial-topology-manager
  • /test pull-kubernetes-node-kubelet-serial-hugepages
  • /test pull-kubernetes-node-crio-cgrpv2-e2e
  • /test pull-kubernetes-node-crio-e2e
  • /test pull-kubernetes-node-kubelet-serial-memory-manager
  • /test pull-kubernetes-e2e-gce-100-performance
  • /test pull-kubernetes-e2e-gce-big-performance
  • /test pull-kubernetes-e2e-gce-correctness
  • /test pull-kubernetes-e2e-gce-large-performance
  • /test pull-kubernetes-kubemark-e2e-gce-big
  • /test pull-kubernetes-kubemark-e2e-gce-scale
  • /test pull-kubernetes-e2e-gce-storage-slow
  • /test pull-kubernetes-e2e-gce-storage-snapshot
  • /test pull-kubernetes-e2e-gce-storage-slow-rbe
  • /test pull-kubernetes-e2e-gce-csi-serial
  • /test pull-kubernetes-e2e-gce-iscsi
  • /test pull-kubernetes-e2e-gce-iscsi-serial
  • /test pull-kubernetes-e2e-gce-storage-disruptive
  • /test pull-kubernetes-e2e-aks-engine-azure-windows
  • /test pull-kubernetes-e2e-azure-disk-windows
  • /test pull-kubernetes-e2e-azure-file-windows
  • /test pull-kubernetes-e2e-aks-engine-windows-gpu
  • /test pull-kubernetes-typecheck
  • /test pull-kubernetes-verify
  • /test pull-kubernetes-e2e-windows-gce

Use /test all to run the following jobs:

  • pull-kubernetes-bazel-build
  • pull-kubernetes-bazel-test
  • pull-kubernetes-conformance-kind-ipv6-parallel
  • pull-kubernetes-dependencies
  • pull-kubernetes-e2e-gce-ubuntu-containerd
  • pull-kubernetes-integration
  • pull-kubernetes-e2e-kind
  • pull-kubernetes-e2e-kind-ipv6
  • pull-kubernetes-conformance-kind-ga-only-parallel
  • pull-kubernetes-node-e2e
  • pull-kubernetes-e2e-gce-100-performance
  • pull-kubernetes-typecheck
  • pull-kubernetes-verify

In response to this:

/retest pull-kubernetes-e2e-gce-ubuntu-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.

@hh
Copy link
Member

hh commented Nov 6, 2020

/test pull-kubernetes-e2e-gce-ubuntu-containerd

@riaankleinhans
Copy link
Contributor Author

/unhold

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 6, 2020
@riaankleinhans
Copy link
Contributor Author

/test pull-kubernetes-bazel-test

@heyste
Copy link
Member

heyste commented Nov 7, 2020

/test pull-kubernetes-e2e-gce-ubuntu-containerd
/test pull-kubernetes-bazel-test

@hh
Copy link
Member

hh commented Nov 7, 2020

To give a quick summary of those last two failures:

9 unrelated tests seem to have connection and node failures:

Error from server: Get "https://10.40.0.5:10250/containerLogs/gcp-volume-9890/nfs-server/nfs-server":
dial tcp 10.40.0.5:10250: connect: connection refused
Nov  7 00:43:58.066: failed dialing endpoint (initial), did not find expected responses... 
Message: "error dialing backend: dial tcp 10.40.0.5:10250: i/o timeout",
Message: "an error on the server (\"unknown\") has prevented the request from succeeding (get nodes

Device Manager related:

FAIL: //pkg/kubelet/cm/devicemanager:go_default_test (run 1 of 2)...
INFO: From Testing //pkg/kubelet/cm/devicemanager:go_default_test (run 1 of 2):
I1107 00:43:46.703222  215173 device_plugin_stub.go:142]
  Starting to serve on /tmp/device_plugin095116164/device-plugin.sock.third
--- FAIL: TestDevicePluginReRegistrationProbeMode (1.03s)

@k8s-ci-robot k8s-ci-robot merged commit 4949078 into kubernetes:master Nov 7, 2020
conformance-definition automation moved this from In Progress PRs to Done Nov 7, 2020
@riaankleinhans riaankleinhans moved this from Done to Promotion PRs Needing Two Weeks (flake free) in conformance-definition Nov 7, 2020
@liggitt liggitt moved this from Promotion PRs Needing Two Weeks (flake free) to Done in conformance-definition Nov 17, 2020
This was referenced Mar 22, 2022
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@spiffxp spiffxp spiffxp requested changes

@heyste heyste heyste left review comments

@dashpole dashpole dashpole left review comments

@BobyMCbobs BobyMCbobs BobyMCbobs left review comments

@Random-Liu Random-Liu Awaiting requested review from Random-Liu

Assignees

@hh hh

@spiffxp spiffxp

@smarterclayton smarterclayton

@dashpole dashpole

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/conformance Issues or PRs related to kubernetes conformance tests area/test cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note-none Denotes a PR that doesn't merit a release note. sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
conformance-definition
  
Done
Milestone
v1.20
Development

Successfully merging this pull request may close these issues.

Write CoreV1Pod and CoreV1PodStatus lifecycle test - +4 coverage