Discover Good First Issues for kubernetes/kubernetes
-
Report event for the cases when probe returned Unknown result Created: 2023-02-24T00:02:49Z
There are a few cases when the Unknown result can be returned by the probe. For example see: https://github.com/kubernetes/kubernetes/issues/106682.
It will be useful to expose those in logs and as an event similar to what we do for Warning: https://github.com/kubernetes/kubernetes/blob/35f3fc59c1f29eebc8ff8705ecc3d4db7c4cbbc6/pkg/kubelet/prober/prober.go#L115
What needs to be done:
Implement logging and exposing event in case of
Unknown
probe result somewhere around https://github.com/kubernetes/kubernetes/blob/35f3fc59c1f29eebc8ff8705ecc3d4db7c4cbbc6/pkg/kubelet/prober/prober.go#L115For each instance of probes potentially returning the
Unknown
result have a test that validates this test case. It may be either e2e test or unit test. You may find an example of e2e test that results in Unknown result from exec probe in one of the links from https://github.com/kubernetes/kubernetes/issues/106682
/sig node
/help wanted
/good-first-issue
/priority backlog
This issue is marked as “good first issue”. Before assigning issue to yourself, please make sure you understand the assignment and list cases you will need to cover with tests.
cc @aojea
Comments: 23
-
Reuse the http request object for http probes Created: 2023-02-21T23:07:09Z
Today we construct the request object for every http request for the http probes. This involves concatenating some strings, setting headers, etc.
Probes are simple http get requests that are executed synchronously (for a single probe) and has no request body.
The optimization may be to reuse the request object for all probe executions. This likely save some cpu and memory for the kubelet process.
Some code references:
DoProbe is executed today on each probe execution.
New request are being created for each execution.
New object will need to be stored in worker that for http probes will hold the http request object. This object will need to encapsulate the logic of runProbe method of the prober.
I’m marking this issue as a good first issue, as the issue doesn’t require deep knowledge of Kubernetes. However it is not an easy issue, a few parts of code will be affected and change needs to be done very carefully.
Please review code and make sure you understand the task before self-assigning the issue.
Implementation notes
If you can send the small benchmark results on probes with and without this optimization, it will be a good starting point. Note, for the benchmark, each probe can run once a second, not more often. But the number of probes is virtually unlimited since we are not defining the limit on number of containers per Pod. Realistically we are talking about 110 pods (max) with maybe 2 containers each, with 3 probes each (660 probes).
Code change may be big. So make sure to help reviewers to review the code. One idea may be to split implementation into multiple commits to simplify the review. First, creating the object encapsulating the logic of
runProbe
, second storing and initializing this object inworker
, and third, actual reusing of the request object.
/sig node
/good-first-issue
/help-wanted
/area kubelet
Comments: 24
-
Give an indication in container events for probe failure as to whether the failure was ignored due to FailureThreshold Created: 2023-02-16T09:02:13Z
Probes of all kinds currently support FailureThreshold (and SuccessThreshold), these properties allow a user to specify that Kubernetes should not take action in response to a failed probe unless it fails a successive number of times.
This is useful for end-users as it allows them to mitigate the effects of any probes that “flake” by requiring successive failure.
When a probe fails in Kubernetes, we emit a container event indicating this here: https://github.com/kubernetes/kubernetes/blob/master/pkg/kubelet/prober/prober.go#L110 and end-users can consume these events via the API for their own purposes. This event is emitted regardless of whether the FailureThreshold has been reached or not.
Currently when a user consumes a probe failure event they have no way of knowing whether the event resulted in action on the control plane (because the event can be ignored due to FailureThreshold, and information on this is not included in the event). This can lead to users assuming there is a problem and a container/pod was restarted when nothing occurred.
I think we should expose the keepGoing value from https://github.com/kubernetes/kubernetes/blob/master/pkg/kubelet/prober/worker.go#L203 in the emitted event somehow, my preferred solution is to emit the probe failure event in the worker rather than where it currently sits in https://github.com/kubernetes/kubernetes/blob/master/pkg/kubelet/prober/prober.go#L110 - there is also the option of passing some information down the stack into the prober from the worker (such as making the FailureThreshold/SuccessThreshold decision in the prober) but I’m worried about separation of concerns, happy to hear what other folks think :)
Also of note is that FailureThreshold/SuccessThreshold is the only filter I can see where a probe can be ignored after being run (and therefore emitting a container event)
I’m happy to write this PR once we’re confident in our approach :)
Comments: 14
-
Write the stress test for gRPC, http, and tcp probes Created: 2023-02-15T00:07:14Z
Write a new e2e test that will be marked as
[Serial]
and will be covering this bug fix: https://github.com/kubernetes/kubernetes/issues/89898The test will serve as a replacement for the skipped unit test: https://github.com/kubernetes/kubernetes/pull/115329
The logic of the test will be as following:
Create MANY containers (see the unit test logic)
Each container has a liveness probe with
1s
intervalAfter some time validate that neither of containers has
restartCount > 0
/cc @aojea
/sig node
/kind cleanup
/priority important-soon
/area tests
/help-wanted
/good-first-issue
This is part of KEP: https://github.com/kubernetes/enhancements/issues/2727 GA requirements, but also need the same tests for http and tcp.
Comments: 20
-
NetworkPolicy tests for blocking north/south traffic Created: 2022-12-08T16:32:02Z
The NP docs point out that NP’s semantics for north/south traffic are not very clearly defined:
Cluster ingress and egress mechanisms often require rewriting the source or destination IP of packets. In cases where this happens, it is not defined whether this happens before or after NetworkPolicy processing, and the behavior may be different for different combinations of network plugin, cloud provider,
Service
implementation, etc.In the case of ingress, this means that in some cases you may be able to filter incoming packets based on the actual original source IP, while in other cases, the “source IP” that the NetworkPolicy acts on may be the IP of a
LoadBalancer
or of the Pod’s node, etc.For egress, this means that connections from pods to
Service
IPs that get rewritten to cluster-external IPs may or may not be subject toipBlock
-based policies.However, what’s not ambiguous is that a pod which is fully isolated for ingress should not accept E/W or N/S traffic. Regardless of how the ingress/LB/cloud handles the traffic, it should end up getting blocked. (Well, unless it gets masqueraded to the pod’s node IP, because then it hits that exception to the rules.)
Unfortunately we can’t say the same for egress; I think we assume that a pod-to-service-IP connection will be allowed to reach kube-proxy even if the pod is fully isolated-for-egress, but we explicitly don’t require that
ipBlock
policies get applied after service proxying.Anyway, we should be able to add a test to
test/e2e/network/netpol/network_policy.go
that confirms that cluster-ingress traffic to a fully isolated-for-ingress pod is not allowed. In particular, if we create a LoadBalancer Service withexternalTrafficPolicy: Local
, and a NetworkPolicy blocking all ingress to that Service’s Pods, then we should not be able to connect to the service via either the LoadBalancer IP/name or via its NodePort (on one of the correct nodes)./sig network
/area network-policy
/priority backlog
/help
/good-first-issue
Comments: 23
-
Node lifecycle controller does not `markPodsNotReady` when the node `Ready` state changes from `false` to `unknown` Created: 2022-09-26T12:05:17Z
What happened?
When kubelet loses connect, the node goes into the unknown state. The node lifecycle controller marks the pod as not ready by the
markPodsNotReady
function because the health check status of the pod can not be obtained through kubelet. This feature is available only when node’sReady
state transitions fromtrue
tounknown
.However, if the node is already in the fail state (such as a containerd failure),
markPodsNotReady
will not take effect if the node loses its connection at this time.In this case, the pod may accidentally remain ready, which may cause some network traffic to be accidentally forwarded to this node.
What did you expect to happen?
As long as the node loses its connection beyond grace time,
MarkPodsNotReady
should always workHow can we reproduce it (as minimally and precisely as possible)?
Stop containerd and wait for the node
Ready
state to falseStop kubelet or shutdown the node and wait the node
Ready
state to unknownThe pods which not be evicted on this node would be always ready
Anything else we need to know?
In the node lifecycle controller logic,
MarkPodsNotReady
is just triggered when a node goes fromtrue
state to anunknown
state. The correct way is to trigger when the node becomesunknown
state regardless of whether the node state was previously trueKubernetes version
$ kubectl version Server Version: version.Info{Major:"1", Minor:"22", GitVersion:"v1.22.15", GitCommit:"1d79bc3bcccfba7466c44cc2055d6e7442e140ea", GitTreeState:"clean", BuildDate:"2022-09-22T06:03:36Z", GoVersion:"go1.16.15", Compiler:"gc", Platform:"linux/amd64"}
Cloud provider
OS version
# On Linux: $ cat /etc/os-release $ uname -a 5.4.119-1-tlinux4-0008 #1 SMP Fri Nov 26 11:17:45 CST 2021 x86_64 x86_64 x86_64 GNU/Linux # On Windows: C:\> wmic os get Caption, Version, BuildNumber, OSArchitecture # paste output here
Install tools
Container runtime (CRI) and version (if applicable)
Related plugins (CNI, CSI, …) and versions (if applicable)
Comments: 21
-
kubectl apply: prune should ignore no-namespaced resource if -n is not empty Created: 2022-07-01T07:34:29Z
kubectl apply
with below yaml and follow below prune command.** namespace sandbox ** --- apiVersion: v1 kind: Service metadata: annotations: environment: sandbox provider: kustomize labels: app: nginx name: nginx namespace: sandbox spec: ports: - port: 80 selector: app: nginx type: NodePort
seems I have the same problem. save this to a file named
a.yaml
apiVersion: v1 kind: Service metadata: annotations: environment: sandbox provider: kustomize labels: app: nginx name: nginx namespace: sandbox spec: ports: - port: 80 selector: app: nginx type: NodePort
And run
kubectl apply --prune --dry-run=client --all -n sandbox -f a.yaml
, the resultsservice/nginx created (dry run) namespace/cattle-system pruned (dry run)
I have no idea why it want to prune the
namespace/cattle-system
.Originally posted by @wd in https://github.com/kubernetes/kubernetes/issues/66430#issuecomment-708922169
Comments: 28
-
[FG:InPlacePodVerticalScaling] Add E2E test case to revert resource resize patch Created: 2022-05-09T13:50:31Z
What would you like to be added?
Pod resize E2E tests need a test case to revert the applied patch and verify that the original values are restored. PR discussion: https://github.com/kubernetes/kubernetes/pull/102884#discussion_r865662604
This should be done for both cases:
- Fully actuated and resized resources are reflected in pod status.
- Fully actuated but not yet reflected in pod status.
Why is this needed?
This verifies that actual.resources == desired.resources in instances where a resize is backed out after it has been partially actuated.
Comments: 25
-
tracker: improve the kubelet test coverage Created: 2022-04-29T06:51:28Z
There is a strong and growing consensus in sig-node about the need of increasing the test coverage in the kubelet, improving the current testsuite about coverage and reliability. See for example the April 26 sig-node meeting notes and the April 27 sig-node CI subgroup meeting notes.
This work has already begun and there are already some PR posted (see: https://github.com/kubernetes/kubernetes/pull/108024#pullrequestreview-954083698). This is a great start. There are more areas in the kubelet, especially in the container manager and resource manager area, that can use better unit test coverage.
Besides the obvious benefits of documenting and preserving the current behaviour, adding tests is meant to lower the barrier for future work and contributions.
Examples of some areas which can benefit of more tests:
k/k/pkg/kubelet/cm/internal_container_lifecycle.go
k/k/pkg/kubelet/cm/internal_container_lifecycle_linux.go
k/k/pkg/kubelet/cm/pod_container_manager_linux.go
k/k/pkg/kubelet/cm/qos_container_manager_linux.go
everything coverage-driven (check test code coverage and build from there)
error paths in general
e2e tests in general
Comments: 37
-
go vet lostcancel errors in legacy-cloud-providers files Created: 2022-04-01T10:44:47Z
What happened?
We found go vet issues of
lostcancel
in #109184. (It seems that go vet checks are not running under/staging
dir.)staging/src/k8s.io/legacy-cloud-providers/vsphere/nodemanager.go:190:5: lostcancel: the cancel function is not used on all paths (possible context leak) (govet) ctx, cancel := context.WithCancel(context.Background()) ^ staging/src/k8s.io/legacy-cloud-providers/vsphere/nodemanager.go:236:3: lostcancel: this return statement may be reached without using the cancel var defined on line 190 (govet) }()
We should fix them to ensure we don’t introduce context leak as the message said.
What did you expect to happen?
These issues from go vet are fixed.
How can we reproduce it (as minimally and precisely as possible)?
Running go vet under
staging/src/k8s.io/legacy-cloud-providers/
Anything else we need to know?
No response
Kubernetes version
latest master
Cloud provider
OS version
# On Linux: $ cat /etc/os-release # paste output here $ uname -a # paste output here # On Windows: C:\> wmic os get Caption, Version, BuildNumber, OSArchitecture # paste output here
Install tools
Container runtime (CRI) and version (if applicable)
Related plugins (CNI, CSI, …) and versions (if applicable)
Comments: 15
-
TOB-K8S-004: Pervasive world-accessible file permissions Created: 2019-08-08T02:03:10Z
This issue was reported in the Kubernetes Security Audit Report
Description
Kubernetes uses files and directories to store information ranging from key-value data to certificate data to logs. However, a number of locations have world-writable directories:
cluster/images/etcd/migrate/rollback_v2.go:110: if err := os.MkdirAll(path.Join(migrateDatadir, "member", "snap"), 0777); err != nil { cluster/images/etcd/migrate/data_dir.go:49: err := os.MkdirAll(path, 0777) cluster/images/etcd/migrate/data_dir.go:87: err = os.MkdirAll(backupDir, 0777) third_party/forked/godep/save.go:472: err := os.MkdirAll(filepath.Dir(dst), 0777) third_party/forked/godep/save.go:585: err := os.MkdirAll(filepath.Dir(name), 0777) pkg/volume/azure_file/azure_util.go:34: defaultFileMode = "0777" pkg/volume/azure_file/azure_util.go:35: defaultDirMode = "0777" pkg/volume/emptydir/empty_dir.go:41:const perm os.FileMode = 0777
Figure 7.1: World-writable (0777) directories and defaults
Other areas of the system use world-writable files as well:
cluster/images/etcd/migrate/data_dir.go:147: return ioutil.WriteFile(v.path, data, 0666) cluster/images/etcd/migrate/migrator.go:120: err := os.Mkdir(backupDir, 0666) third_party/forked/godep/save.go:589: return ioutil.WriteFile(name, []byte(body), 0666) pkg/kubelet/kuberuntime/kuberuntime_container.go:306: if err := m.osInterface.Chmod(containerLogPath, 0666); err != nil { pkg/volume/cinder/cinder_util.go:271: ioutil.WriteFile(name, data, 0666) pkg/volume/fc/fc_util.go:118: io.WriteFile(fileName, data, 0666) pkg/volume/fc/fc_util.go:128: io.WriteFile(name, data, 0666) pkg/volume/azure_dd/azure_common_linux.go:77: if err = io.WriteFile(name, data, 0666); err != nil { pkg/volume/photon_pd/photon_util.go:55: ioutil.WriteFile(fileName, data, 0666) pkg/volume/photon_pd/photon_util.go:65: ioutil.WriteFile(name, data, 0666)
Figure 7.2: World-writable (0666) files
A number of locations in the code base also rely on world-readable directories and files. For example, Certificate Signing Requests (CSRs) are written to a directory with mode 0755 (world readable and browseable) with the actual CSR having mode 0644 (world-readable):
// WriteCSR writes the pem-encoded CSR data to csrPath. // The CSR file will be created with file mode 0644. // If the CSR file already exists, it will be overwritten. // The parent directory of the csrPath will be created as needed with file mode 0755. func WriteCSR(csrDir, name string, csr *x509.CertificateRequest) error { ... if err := os.MkdirAll(filepath.Dir(csrPath), os.FileMode(0755)); err != nil { ... } if err := ioutil.WriteFile(csrPath, EncodeCSRPEM(csr), os.FileMode(0644)); err != nil { ... } ... }
Figure 7.3: Documentation and code from cmd/kubeadm/app/util/pkiutil/pki_helpers.go
Exploit Scenario
Alice wishes to migrate some etcd values during normal cluster maintenance. Eve has local access to the cluster’s filesystem, and modifies the values stored during the migration process, granting Eve further access to the cluster as a whole.
Recommendation
Short term, audit all locations that use world-accessible permissions. Revoke those that are unnecessary. Very few files truly need to be readable by any user on a system. Almost none should need to allow arbitrary system users write access.
Long term, use system groups and extended Access Control Lists (ACLs) to ensure that all files and directories created by Kuberenetes are accessible by only those users and groups that should be able to access them. This will ensure that only the appropriate users with the correct Unix-level groups may access data. Kubernetes may describe what these groups should be, or create a role-based system to which administrators may assign users and groups.
Anything else we need to know?:
See #81146 for current status of all issues created from these findings.
The vendor gave this issue an ID of TOB-K8S-004 and it was finding 8 of the report.
The vendor considers this issue Medium Severity.
To view the original finding, begin on page 32 of the Kubernetes Security Review Report
Environment:
- Kubernetes version: 1.13.4
Comments: 36
-
`failed to garbage collect required amount of images. Wanted to free 473842483 bytes, but freed 0 bytes` Created: 2018-12-08T00:21:28Z
What happened: I’ve been seeing a number of evictions recently that appear to be due to disk pressure:
$$$ kubectl get pod kumo-go-api-d46f56779-jl6s2 --namespace=kumo-main -o yaml apiVersion: v1 kind: Pod metadata: creationTimestamp: 2018-12-06T10:05:25Z generateName: kumo-go-api-d46f56779- labels: io.kompose.service: kumo-go-api pod-template-hash: "802912335" name: kumo-go-api-d46f56779-jl6s2 namespace: kumo-main ownerReferences: - apiVersion: extensions/v1beta1 blockOwnerDeletion: true controller: true kind: ReplicaSet name: kumo-go-api-d46f56779 uid: c0a9355e-f780-11e8-b336-42010aa80057 resourceVersion: "11617978" selfLink: /api/v1/namespaces/kumo-main/pods/kumo-go-api-d46f56779-jl6s2 uid: 7337e854-f93e-11e8-b336-42010aa80057 spec: containers: - env: - redacted... image: gcr.io/<redacted>/kumo-go-api@sha256:c6a94fc1ffeb09ea6d967f9ab14b9a26304fa4d71c5798acbfba5e98125b81da imagePullPolicy: Always name: kumo-go-api ports: - containerPort: 5000 protocol: TCP resources: {} terminationMessagePath: /dev/termination-log terminationMessagePolicy: File volumeMounts: - mountPath: /var/run/secrets/kubernetes.io/serviceaccount name: default-token-t6jkx readOnly: true dnsPolicy: ClusterFirst nodeName: gke-kumo-customers-n1-standard-1-pree-0cd7990c-jg9s restartPolicy: Always schedulerName: default-scheduler securityContext: {} serviceAccount: default serviceAccountName: default terminationGracePeriodSeconds: 30 tolerations: - effect: NoExecute key: node.kubernetes.io/not-ready operator: Exists tolerationSeconds: 300 - effect: NoExecute key: node.kubernetes.io/unreachable operator: Exists tolerationSeconds: 300 volumes: - name: default-token-t6jkx secret: defaultMode: 420 secretName: default-token-t6jkx status: message: 'The node was low on resource: nodefs.' phase: Failed reason: Evicted startTime: 2018-12-06T10:05:25Z
Taking a look at
kubectl get events
, I see these warnings:$$$ kubectl get events LAST SEEN FIRST SEEN COUNT NAME KIND SUBOBJECT TYPE REASON SOURCE MESSAGE 2m 13h 152 gke-kumo-customers-n1-standard-1-pree-0cd7990c-jg9s.156e07f40b90ed91 Node Warning ImageGCFailed kubelet, gke-kumo-customers-n1-standard-1-pree-0cd7990c-jg9s (combined from similar events): failed to garbage collect required amount of images. Wanted to free 473948979 bytes, but freed 0 bytes 37m 37m 1 gke-kumo-customers-n1-standard-1-pree-0cd7990c-jg9s.156e3127ebc715c3 Node Warning ImageGCFailed kubelet, gke-kumo-customers-n1-standard-1-pree-0cd7990c-jg9s failed to garbage collect required amount of images. Wanted to free 473674547 bytes, but freed 0 bytes
Digging a bit deeper:
$$$ kubectl get event gke-kumo-customers-n1-standard-1-pree-0cd7990c-jg9s.156e07f40b90ed91 -o yaml apiVersion: v1 count: 153 eventTime: null firstTimestamp: 2018-12-07T11:01:06Z involvedObject: kind: Node name: gke-kumo-customers-n1-standard-1-pree-0cd7990c-jg9s uid: gke-kumo-customers-n1-standard-1-pree-0cd7990c-jg9s kind: Event lastTimestamp: 2018-12-08T00:16:09Z message: '(combined from similar events): failed to garbage collect required amount of images. Wanted to free 474006323 bytes, but freed 0 bytes' metadata: creationTimestamp: 2018-12-07T11:01:07Z name: gke-kumo-customers-n1-standard-1-pree-0cd7990c-jg9s.156e07f40b90ed91 namespace: default resourceVersion: "381976" selfLink: /api/v1/namespaces/default/events/gke-kumo-customers-n1-standard-1-pree-0cd7990c-jg9s.156e07f40b90ed91 uid: 65916e4b-fa0f-11e8-ae9a-42010aa80058 reason: ImageGCFailed reportingComponent: "" reportingInstance: "" source: component: kubelet host: gke-kumo-customers-n1-standard-1-pree-0cd7990c-jg9s type: Warning
There’s actually remarkably little here. This message doesn’t say anything regarding why ImageGC was initiated or why it was unable recover more space.
What you expected to happen: Image GC to work correctly, or at least fail to schedule pods onto nodes that do not have sufficient disk space.
How to reproduce it (as minimally and precisely as possible): Run and stop as many pods as possible on a node in order to encourage disk pressure. Then observe these errors.
Anything else we need to know?: n/a
Environment:
- Kubernetes version (use
kubectl version
):
Client Version: version.Info{Major:"1", Minor:"10", GitVersion:"v1.10.7", GitCommit:"0c38c362511b20a098d7cd855f1314dad92c2780", GitTreeState:"clean", BuildDate:"2018-08-20T10:09:03Z", GoVersion:"go1.9.3", Compiler:"gc", Platform:"darwin/amd64"} Server Version: version.Info{Major:"1", Minor:"10+", GitVersion:"v1.10.7-gke.11", GitCommit:"fa90543563c9cfafca69128ce8cd9ecd5941940f", GitTreeState:"clean", BuildDate:"2018-11-08T20:22:21Z", GoVersion:"go1.9.3b4", Compiler:"gc", Platform:"linux/amd64"}
Cloud provider or hardware configuration: GKE
OS (e.g. from /etc/os-release): I’m running macOS 10.14, nodes are running Container-Optimized OS (cos).
Kernel (e.g.
uname -a
):Darwin D-10-19-169-80.dhcp4.washington.edu 18.0.0 Darwin Kernel Version 18.0.0: Wed Aug 22 20:13:40 PDT 2018; root:xnu-4903.201.2~1/RELEASE_X86_64 x86_64
Install tools: n/a
Others: n/a
/kind bug
Comments: 68
- Kubernetes version (use
-
Figure out a way to manage the discrepancy on windows nodes from the linux node Created: 2018-02-24T01:15:29Z
I am expecting we have more windows specific config like code coming to Kubelet and other node components. We should figure out a way to handle this, so that the code can be easily managed? Instead of letting windows-special handling code like this scattered everywhere in our codebase.
cc/ @feiskyer @michmike @kubernetes/sig-windows-bugs
Comments: 21
-
Move selector immutability check to validation after v1beta1 retires Created: 2017-08-16T20:11:30Z
The check for selector immutability is located at
PrepareForUpdate
functions ofdeploymentStrategy
,rsStrategy
anddaemonSetStrategy
. We are not able to have the check at validation before v1beta1 API version retires due to breaking change to some tests (discussed in this closed PR). Once v1beta1 retires, we should move the check to validation.Comments: 12