volcano-sh · volcano-sh-bot · Dec 14, 2019 · Dec 12, 2019
diff --git a/installer/helm/chart/volcano/templates/controllers.yaml b/installer/helm/chart/volcano/templates/controllers.yaml
@@ -40,6 +40,9 @@ rules:
   - apiGroups: [""]
     resources: ["configmaps"]
     verbs: ["get", "list", "watch", "create", "delete", "update"]
+  - apiGroups: [""]
+    resources: ["secrets"]
+    verbs: ["get", "list", "watch", "create", "delete", "update"]
   - apiGroups: ["scheduling.incubator.k8s.io", "scheduling.sigs.dev"]
     resources: ["podgroups", "queues", "queues/status"]
     verbs: ["get", "list", "watch", "create", "delete", "update"]

diff --git a/installer/volcano-development.yaml b/installer/volcano-development.yaml
@@ -315,6 +315,9 @@ rules:
   - apiGroups: [""]
     resources: ["configmaps"]
     verbs: ["get", "list", "watch", "create", "delete", "update"]
+  - apiGroups: [""]
+    resources: ["secrets"]
+    verbs: ["get", "list", "watch", "create", "delete", "update"]
   - apiGroups: ["scheduling.incubator.k8s.io", "scheduling.sigs.dev"]
     resources: ["podgroups", "queues", "queues/status"]
     verbs: ["get", "list", "watch", "create", "delete", "update"]

diff --git a/pkg/apis/helpers/helpers.go b/pkg/apis/helpers/helpers.go
@@ -122,6 +122,24 @@ func CreateConfigMapIfNotExist(job *vcbatch.Job, kubeClients kubernetes.Interfac
 	return nil
 }
 
+// CreateSecret create secret
+func CreateSecret(job *vcbatch.Job, kubeClients kubernetes.Interface, data map[string][]byte, secretName string) error {
+	secret := &v1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      secretName,
+			Namespace: job.Namespace,
+			OwnerReferences: []metav1.OwnerReference{
+				*metav1.NewControllerRef(job, JobKind),
+			},
+		},
+		Data: data,
+	}
+
+	_, err := kubeClients.CoreV1().Secrets(job.Namespace).Create(secret)
+
+	return err
+}
+
 // DeleteConfigmap  deletes the config map resource
 func DeleteConfigmap(job *vcbatch.Job, kubeClients kubernetes.Interface, cmName string) error {
 	if _, err := kubeClients.CoreV1().ConfigMaps(job.Namespace).Get(cmName, metav1.GetOptions{}); err != nil {
@@ -145,6 +163,16 @@ func DeleteConfigmap(job *vcbatch.Job, kubeClients kubernetes.Interface, cmName
 	return nil
 }
 
+// DeleteSecret delete secret
+func DeleteSecret(job *vcbatch.Job, kubeClients kubernetes.Interface, secretName string) error {
+	err := kubeClients.CoreV1().Secrets(job.Namespace).Delete(secretName, nil)
+	if err != nil && true == apierrors.IsNotFound(err) {
+		return nil
+	}
+
+	return err
+}
+
 // GeneratePodgroupName  generate podgroup name of normal pod
 func GeneratePodgroupName(pod *v1.Pod) string {
 	pgName := vcbatch.PodgroupNamePrefix

diff --git a/pkg/controllers/job/job_controller_actions_test.go b/pkg/controllers/job/job_controller_actions_test.go
@@ -42,6 +42,7 @@ func TestKillJobFunc(t *testing.T) {
 		JobInfo        *apis.JobInfo
 		Services       []v1.Service
 		ConfigMaps     []v1.ConfigMap
+		Secrets        []v1.Secret
 		Pods           map[string]*v1.Pod
 		Plugins        []string
 		ExpextVal      error
@@ -52,6 +53,7 @@ func TestKillJobFunc(t *testing.T) {
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "job1",
 					Namespace: namespace,
+					UID:       "e7f18111-1cec-11ea-b688-fa163ec79500",
 				},
 			},
 			PodGroup: &schedulingv1alpha2.PodGroup{
@@ -80,10 +82,10 @@ func TestKillJobFunc(t *testing.T) {
 					},
 				},
 			},
-			ConfigMaps: []v1.ConfigMap{
+			Secrets: []v1.Secret{
 				{
 					ObjectMeta: metav1.ObjectMeta{
-						Name:      "job1-ssh",
+						Name:      "job1-e7f18111-1cec-11ea-b688-fa163ec79500-ssh",
 						Namespace: namespace,
 					},
 				},
@@ -110,10 +112,10 @@ func TestKillJobFunc(t *testing.T) {
 				}
 			}
 
-			for _, configMap := range testcase.ConfigMaps {
-				_, err := fakeController.kubeClient.CoreV1().ConfigMaps(namespace).Create(&configMap)
+			for _, secret := range testcase.Secrets {
+				_, err := fakeController.kubeClient.CoreV1().Secrets(namespace).Create(&secret)
 				if err != nil {
-					t.Error("Error While Creating ConfigMaps")
+					t.Error("Error While Creating Secret.")
 				}
 			}
 
@@ -142,26 +144,26 @@ func TestKillJobFunc(t *testing.T) {
 
 			err = fakeController.killJob(testcase.JobInfo, testcase.PodRetainPhase, testcase.UpdateStatus)
 			if err != nil {
-				t.Errorf("Case %d (%s): expected: No Error, but got error %s", i, testcase.Name, err.Error())
+				t.Errorf("Case %d (%s): expected: No Error, but got error %v.", i, testcase.Name, err)
 			}
 
 			for _, plugin := range testcase.Plugins {
 
 				if plugin == "svc" {
 					_, err = fakeController.kubeClient.CoreV1().Services(namespace).Get(testcase.Job.Name, metav1.GetOptions{})
 					if err == nil {
-						t.Errorf("Case %d (%s): expected: Service to be deleted, but not deleted because of error %s", i, testcase.Name, err.Error())
+						t.Errorf("Case %d (%s): expected: Service to be deleted, but not deleted.", i, testcase.Name)
 					}
 				}
 
 				if plugin == "ssh" {
-					_, err := fakeController.kubeClient.CoreV1().ConfigMaps(namespace).Get(fmt.Sprint(testcase.Job.Name, "-ssh"), metav1.GetOptions{})
+					_, err := fakeController.kubeClient.CoreV1().ConfigMaps(namespace).Get(
+						fmt.Sprintf("%s-%s-%s", testcase.Job.Name, testcase.Job.UID, "ssh"), metav1.GetOptions{})
 					if err == nil {
-						t.Errorf("Case %d (%s): expected: ConfigMap to be deleted, but not deleted because of error %s", i, testcase.Name, err.Error())
+						t.Errorf("Case %d (%s): expected: Secret to be deleted, but not deleted.", i, testcase.Name)
 					}
 				}
 			}
-
 		})
 	}
 }

diff --git a/pkg/controllers/job/job_controller_plugins_test.go b/pkg/controllers/job/job_controller_plugins_test.go
@@ -55,6 +55,7 @@ func TestPluginOnPodCreate(t *testing.T) {
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "Job1",
 					Namespace: namespace,
+					UID:       "e7f18111-1cec-11ea-b688-fa163ec79500",
 				},
 			},
 			Pod:     buildPod(namespace, "pod1", v1.PodPending, nil),
@@ -66,6 +67,7 @@ func TestPluginOnPodCreate(t *testing.T) {
 			Job: &batch.Job{
 				ObjectMeta: metav1.ObjectMeta{
 					Name: "Job1",
+					UID:  "e7f18111-1cec-11ea-b688-fa163ec79500",
 				},
 			},
 			Pod:     buildPod(namespace, "pod1", v1.PodPending, nil),
@@ -124,7 +126,7 @@ func TestPluginOnPodCreate(t *testing.T) {
 						}
 						exist := false
 						for _, volume := range container.VolumeMounts {
-							if volume.Name == fmt.Sprint(testcase.Job.Name, "-ssh") {
+							if volume.Name == fmt.Sprintf("%s-%s-%s", testcase.Job.Name, testcase.Job.UID, "ssh") {
 								exist = true
 							}
 						}
@@ -153,6 +155,7 @@ func TestPluginOnJobAdd(t *testing.T) {
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "job1",
 					Namespace: namespace,
+					UID:       "e7f18111-1cec-11ea-b688-fa163ec79500",
 				},
 			},
 			Plugins: []string{"svc", "ssh", "env"},
@@ -163,6 +166,7 @@ func TestPluginOnJobAdd(t *testing.T) {
 			Job: &batch.Job{
 				ObjectMeta: metav1.ObjectMeta{
 					Name: "Job1",
+					UID:  "e7f18111-1cec-11ea-b688-fa163ec79500",
 				},
 			},
 			Plugins: []string{"new"},
@@ -202,9 +206,10 @@ func TestPluginOnJobAdd(t *testing.T) {
 				}
 
 				if plugin == "ssh" {
-					_, err := fakeController.kubeClient.CoreV1().ConfigMaps(namespace).Get(fmt.Sprint(testcase.Job.Name, "-ssh"), metav1.GetOptions{})
+					_, err := fakeController.kubeClient.CoreV1().Secrets(namespace).Get(
+						fmt.Sprintf("%s-%s-%s", testcase.Job.Name, testcase.Job.UID, "ssh"), metav1.GetOptions{})
 					if err != nil {
-						t.Errorf("Case %d (%s): expected: ConfigMap to be created, but not created because of error %s", i, testcase.Name, err.Error())
+						t.Errorf("Case %d (%s): expected: Secret to be created, but not created because of error %s", i, testcase.Name, err.Error())
 					}
 				}
 
@@ -233,6 +238,7 @@ func TestPluginOnJobDelete(t *testing.T) {
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "job1",
 					Namespace: namespace,
+					UID:       "e7f18111-1cec-11ea-b688-fa163ec79500",
 				},
 			},
 			Plugins: []string{"svc", "ssh", "env"},
@@ -243,6 +249,7 @@ func TestPluginOnJobDelete(t *testing.T) {
 			Job: &batch.Job{
 				ObjectMeta: metav1.ObjectMeta{
 					Name: "Job1",
+					UID:  "e7f18111-1cec-11ea-b688-fa163ec79500",
 				},
 			},
 			Plugins: []string{"new"},
@@ -272,23 +279,23 @@ func TestPluginOnJobDelete(t *testing.T) {
 				if plugin == "svc" {
 					_, err := fakeController.kubeClient.CoreV1().ConfigMaps(namespace).Get(fmt.Sprint(testcase.Job.Name, "-svc"), metav1.GetOptions{})
 					if err == nil {
-						t.Errorf("Case %d (%s): expected: ConfigMap to be deleted, but not deleted because of error %s", i, testcase.Name, err.Error())
+						t.Errorf("Case %d (%s): expected: ConfigMap to be deleted, but not deleted.", i, testcase.Name)
 					}
 
 					_, err = fakeController.kubeClient.CoreV1().Services(namespace).Get(testcase.Job.Name, metav1.GetOptions{})
 					if err == nil {
-						t.Errorf("Case %d (%s): expected: Service to be deleted, but not deleted because of error %s", i, testcase.Name, err.Error())
+						t.Errorf("Case %d (%s): expected: Service to be deleted, but not deleted.", i, testcase.Name)
 					}
 				}
 
 				if plugin == "ssh" {
-					_, err := fakeController.kubeClient.CoreV1().ConfigMaps(namespace).Get(fmt.Sprint(testcase.Job.Name, "-ssh"), metav1.GetOptions{})
+					_, err := fakeController.kubeClient.CoreV1().Secrets(namespace).Get(
+						fmt.Sprintf("%s-%s-%s", testcase.Job.Name, testcase.Job.UID, "ssh"), metav1.GetOptions{})
 					if err == nil {
-						t.Errorf("Case %d (%s): expected: ConfigMap to be deleted, but not deleted because of error %s", i, testcase.Name, err.Error())
+						t.Errorf("Case %d (%s): expected: secret to be deleted, but not deleted.", i, testcase.Name)
 					}
 				}
 			}
 		})
-
 	}
 }
diff --git a/pkg/controllers/job/plugins/ssh/ssh.go b/pkg/controllers/job/plugins/ssh/ssh.go
@@ -84,8 +84,9 @@ func (sp *sshPlugin) OnJobAdd(job *batch.Job) error {
 		return err
 	}
 
-	if err := helpers.CreateConfigMapIfNotExist(job, sp.Clientset.KubeClients, data, sp.cmName(job)); err != nil {
-		return err
+	if err := helpers.CreateSecret(job, sp.Clientset.KubeClients, data, sp.secretName(job)); err != nil {
+		return fmt.Errorf("create secret for job <%s/%s> with ssh plugin failed for %v",
+			job.Namespace, job.Name, err)
 	}
 
 	job.Status.ControlledResources["plugin-"+sp.Name()] = sp.Name()
@@ -94,24 +95,19 @@ func (sp *sshPlugin) OnJobAdd(job *batch.Job) error {
 }
 
 func (sp *sshPlugin) OnJobDelete(job *batch.Job) error {
-	if err := helpers.DeleteConfigmap(job, sp.Clientset.KubeClients, sp.cmName(job)); err != nil {
-		return err
-	}
-
-	return nil
+	return helpers.DeleteSecret(job, sp.Clientset.KubeClients, sp.secretName(job))
 }
 
 func (sp *sshPlugin) mountRsaKey(pod *v1.Pod, job *batch.Job) {
+	secretName := sp.secretName(job)
 
-	cmName := sp.cmName(job)
 	sshVolume := v1.Volume{
-		Name: cmName,
+		Name: secretName,
 	}
+
 	var mode int32 = 0600
-	sshVolume.ConfigMap = &v1.ConfigMapVolumeSource{
-		LocalObjectReference: v1.LocalObjectReference{
-			Name: cmName,
-		},
+	sshVolume.Secret = &v1.SecretVolumeSource{
+		SecretName: secretName,
 		Items: []v1.KeyToPath{
 			{
 				Key:  SSHPrivateKey,
@@ -135,7 +131,7 @@ func (sp *sshPlugin) mountRsaKey(pod *v1.Pod, job *batch.Job) {
 
 	if sp.sshKeyFilePath != SSHAbsolutePath {
 		var noRootMode int32 = 0755
-		sshVolume.ConfigMap.DefaultMode = &noRootMode
+		sshVolume.Secret.DefaultMode = &noRootMode
 	}
 
 	pod.Spec.Volumes = append(pod.Spec.Volumes, sshVolume)
@@ -144,7 +140,7 @@ func (sp *sshPlugin) mountRsaKey(pod *v1.Pod, job *batch.Job) {
 		vm := v1.VolumeMount{
 			MountPath: sp.sshKeyFilePath,
 			SubPath:   SSHRelativePath,
-			Name:      cmName,
+			Name:      secretName,
 		}
 
 		pod.Spec.Containers[i].VolumeMounts = append(c.VolumeMounts, vm)
@@ -153,7 +149,7 @@ func (sp *sshPlugin) mountRsaKey(pod *v1.Pod, job *batch.Job) {
 	return
 }
 
-func generateRsaKey(job *batch.Job) (map[string]string, error) {
+func generateRsaKey(job *batch.Job) (map[string][]byte, error) {
 	bitSize := 1024
 
 	privateKey, err := rsa.GenerateKey(rand.Reader, bitSize)
@@ -177,16 +173,16 @@ func generateRsaKey(job *batch.Job) (map[string]string, error) {
 	}
 	publicKeyBytes := ssh.MarshalAuthorizedKey(publicRsaKey)
 
-	data := make(map[string]string)
-	data[SSHPrivateKey] = string(privateKeyBytes)
-	data[SSHPublicKey] = string(publicKeyBytes)
-	data[SSHConfig] = generateSSHConfig(job)
+	data := make(map[string][]byte)
+	data[SSHPrivateKey] = privateKeyBytes
+	data[SSHPublicKey] = publicKeyBytes
+	data[SSHConfig] = []byte(generateSSHConfig(job))
 
 	return data, nil
 }
 
-func (sp *sshPlugin) cmName(job *batch.Job) string {
-	return fmt.Sprintf("%s-%s", job.Name, sp.Name())
+func (sp *sshPlugin) secretName(job *batch.Job) string {
+	return fmt.Sprintf("%s-%s-%s", job.Name, job.UID, sp.Name())
 }
 
 func (sp *sshPlugin) addFlags() {

diff --git a/test/e2e/job_plugins.go b/test/e2e/job_plugins.go
@@ -148,8 +148,8 @@ var _ = Describe("Job E2E Test: Test Job Plugins", func() {
 		err := waitJobReady(context, job)
 		Expect(err).NotTo(HaveOccurred())
 
-		pluginName := fmt.Sprintf("%s-ssh", jobName)
-		_, err = context.kubeclient.CoreV1().ConfigMaps(namespace).Get(
+		pluginName := fmt.Sprintf("%s-%s-ssh", jobName, job.UID)
+		_, err = context.kubeclient.CoreV1().Secrets(namespace).Get(
 			pluginName, v1.GetOptions{})
 		Expect(err).NotTo(HaveOccurred())
 
@@ -206,8 +206,8 @@ var _ = Describe("Job E2E Test: Test Job Plugins", func() {
 		err := waitJobReady(context, job)
 		Expect(err).NotTo(HaveOccurred())
 
-		pluginName := fmt.Sprintf("%s-ssh", jobName)
-		_, err = context.kubeclient.CoreV1().ConfigMaps(namespace).Get(
+		pluginName := fmt.Sprintf("%s-%s-ssh", jobName, job.UID)
+		_, err = context.kubeclient.CoreV1().Secrets(namespace).Get(
 			pluginName, v1.GetOptions{})
 		Expect(err).NotTo(HaveOccurred())