[GitHub] [incubator-yunikorn-k8shim] yangwwei commented on a change in pull request #369: [YUNIKORN-1040] add e2e test that re-starts the scheduler pod

[GitBox <gi...@apache.org>]

yangwwei commented on a change in pull request #369:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/369#discussion_r825341689



##########
File path: test/e2e/framework/helpers/k8s/k8s_utils.go
##########
@@ -141,6 +191,61 @@ func (k *KubeCtl) GetPodNamesFromNS(namespace string) ([]string, error) {
 	return s, nil
 }
 
+func (k *KubeCtl) PortForwardPod(req PortForwardAPodRequest) error {
+	path := fmt.Sprintf("/api/v1/namespaces/%s/pods/%s/portforward",
+		req.Pod.Namespace, req.Pod.Name)
+	hostIP := strings.TrimLeft(req.RestConfig.Host, "htps:/")
+
+	transport, upgrader, err := spdy.RoundTripperFor(req.RestConfig)
+	if err != nil {
+		panic(err)
+	}
+
+	dialer := spdy.NewDialer(upgrader, &http.Client{Transport: transport}, http.MethodPost, &url.URL{Scheme: "https", Path: path, Host: hostIP})
+	fw, err := portforward.New(dialer, []string{fmt.Sprintf("%d:%d", req.LocalPort, req.PodPort)}, req.StopCh, req.ReadyCh, req.Streams.Out, req.Streams.ErrOut)
+	if err != nil {
+		return err
+	}
+	return fw.ForwardPorts()
+}
+
+func (k *KubeCtl) PortForwardYkSchedulerPod() {
+	stopCh := make(chan struct{}, 1)
+	readyCh := make(chan struct{})
+	stream := genericclioptions.IOStreams{
+		In:     os.Stdin,
+		Out:    os.Stdout,
+		ErrOut: os.Stderr,
+	}
+
+	go func() {
+		schedulerPodName, err := k.GetSchedulerPod()
+		if err != nil {
+			panic(err)

Review comment:
       Instead of panicking out, when there is an error happened, can we notify the main process and exit with an error? Then the caller can catch this exception and print some more meaningful error messages.

##########
File path: test/e2e/framework/helpers/k8s/k8s_utils.go
##########
@@ -112,6 +138,30 @@ func (k *KubeCtl) GetPod(name, namespace string) (*v1.Pod, error) {
 	return k.clientSet.CoreV1().Pods(namespace).Get(context.TODO(), name, metav1.GetOptions{})
 }
 
+func (k *KubeCtl) GetSchedulerPod() (string, error) {
+	podNameList, err := k.GetPodNamesFromNS(configmanager.YuniKornTestConfig.YkNamespace)
+	if err != nil {
+		return "", err
+	}
+	for _, podName := range podNameList {
+		if strings.Contains(podName, configmanager.YKScheduler) {
+			return podName, nil
+		}
+	}
+	return "", errors.New("YK scheduler pod not found")
+}
+
+func (k *KubeCtl) KillKubectlProcess() error {
+	cmd := exec.Command("pkill", "-SIGINT", "kubectl")

Review comment:
       I noticed the portforward has a [close](https://github.com/kubernetes/client-go/blob/2f52a105e63e9ac7affb1a0f533c62883611ba81/tools/portforward/portforward.go#L412) function, is it possible to leverage that instead of killing kubectl process? The concern is in the future, if the testing env has some other kubectl processes, they could be killed by this as a side effect.

##########
File path: test/e2e/framework/helpers/k8s/k8s_utils.go
##########
@@ -141,6 +191,61 @@ func (k *KubeCtl) GetPodNamesFromNS(namespace string) ([]string, error) {
 	return s, nil
 }
 
+func (k *KubeCtl) PortForwardPod(req PortForwardAPodRequest) error {
+	path := fmt.Sprintf("/api/v1/namespaces/%s/pods/%s/portforward",
+		req.Pod.Namespace, req.Pod.Name)
+	hostIP := strings.TrimLeft(req.RestConfig.Host, "htps:/")
+
+	transport, upgrader, err := spdy.RoundTripperFor(req.RestConfig)
+	if err != nil {
+		panic(err)
+	}
+
+	dialer := spdy.NewDialer(upgrader, &http.Client{Transport: transport}, http.MethodPost, &url.URL{Scheme: "https", Path: path, Host: hostIP})
+	fw, err := portforward.New(dialer, []string{fmt.Sprintf("%d:%d", req.LocalPort, req.PodPort)}, req.StopCh, req.ReadyCh, req.Streams.Out, req.Streams.ErrOut)
+	if err != nil {
+		return err
+	}
+	return fw.ForwardPorts()
+}
+
+func (k *KubeCtl) PortForwardYkSchedulerPod() {
+	stopCh := make(chan struct{}, 1)
+	readyCh := make(chan struct{})
+	stream := genericclioptions.IOStreams{
+		In:     os.Stdin,
+		Out:    os.Stdout,
+		ErrOut: os.Stderr,
+	}
+
+	go func() {
+		schedulerPodName, err := k.GetSchedulerPod()
+		if err != nil {
+			panic(err)
+		}
+		err = k.PortForwardPod(PortForwardAPodRequest{
+			RestConfig: k.kubeConfig,
+			Pod: v1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      schedulerPodName,
+					Namespace: configmanager.YuniKornTestConfig.YkNamespace,
+				},
+			},
+			LocalPort: portForwardPort,
+			PodPort:   portForwardPort,
+			Streams:   stream,
+			StopCh:    stopCh,
+			ReadyCh:   readyCh,
+		})
+		if err != nil {
+			panic(err)

Review comment:
       Same as above




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org