kubectl EvictPod method should use Helper's context

zou2699 commented 3 months ago

What happened?

The EvictPod method in the Helper struct currently uses context.TODO() when making eviction requests, which is inconsistent with the Helper.ctx used throughout the rest of the Helper methods. This inconsistency can lead to issues, particularly when using rest.Wrap, as it prevents accessing values stored in Helper.ctx.

What did you expect to happen?

Helper.EvictPod method should use Helper.ctx.

How can we reproduce it (as minimally and precisely as possible)?

this is exist in code

// EvictPod will evict the given pod, or return an error if it couldn't
func (d *Helper) EvictPod(pod corev1.Pod, evictionGroupVersion schema.GroupVersion) error {
    if d.DryRunStrategy == cmdutil.DryRunServer {
        if err := d.DryRunVerifier.HasSupport(pod.GroupVersionKind()); err != nil {
            return err
        }
    }

    delOpts := d.makeDeleteOptions()

    switch evictionGroupVersion {
    case policyv1.SchemeGroupVersion:
        // send policy/v1 if the server supports it
        eviction := &policyv1.Eviction{
            ObjectMeta: metav1.ObjectMeta{
                Name:      pod.Name,
                Namespace: pod.Namespace,
            },
            DeleteOptions: &delOpts,
        }
        return d.Client.PolicyV1().Evictions(eviction.Namespace).Evict(d.getContext(), eviction)

    default:
        // otherwise, fall back to policy/v1beta1, supported by all servers that support the eviction subresource
        eviction := &policyv1beta1.Eviction{
            ObjectMeta: metav1.ObjectMeta{
                Name:      pod.Name,
                Namespace: pod.Namespace,
            },
            DeleteOptions: &delOpts,
        }
        return d.Client.PolicyV1beta1().Evictions(eviction.Namespace).Evict(d.getContext(), eviction)
    }
}

Anything else we need to know?

No response

Kubernetes version

```console $ kubectl version # paste output here ```

Cloud provider

OS version

```console # 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)

k8s-ci-robot commented 3 months ago

There are no sig labels on this issue. Please add an appropriate label by using one of the following commands:

/sig <group-name>
/wg <group-name>
/committee <group-name>

Please see the group list for a listing of the SIGs, working groups, and committees available.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.

k8s-ci-robot commented 3 months ago

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](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.

ardaguclu commented 3 months ago

/transfer kubectl /sig cli

ardaguclu commented 3 months ago

/triage accepted /priority backlog

kubernetes / kubectl