Skip to content

Commit ad9ea1e

Browse files
volcano-sh-botvolcano-sh-bot
authored
Merge pull request #603 from sivanzcw/dev
change storage of ssh pem from configmap to secret for ssh plugin
2 parents 5348a8d + 5d517cf commit ad9ea1e

7 files changed

Lines changed: 83 additions & 44 deletions

File tree

‎installer/helm/chart/volcano/templates/controllers.yaml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,9 @@ rules:
4040
- apiGroups: [""]
4141
resources: ["configmaps"]
4242
verbs: ["get", "list", "watch", "create", "delete", "update"]
43+
- apiGroups: [""]
44+
resources: ["secrets"]
45+
verbs: ["get", "list", "watch", "create", "delete", "update"]
4346
- apiGroups: ["scheduling.incubator.k8s.io", "scheduling.sigs.dev"]
4447
resources: ["podgroups", "queues", "queues/status"]
4548
verbs: ["get", "list", "watch", "create", "delete", "update"]

‎installer/volcano-development.yaml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,9 @@ rules:
315315
- apiGroups: [""]
316316
resources: ["configmaps"]
317317
verbs: ["get", "list", "watch", "create", "delete", "update"]
318+
- apiGroups: [""]
319+
resources: ["secrets"]
320+
verbs: ["get", "list", "watch", "create", "delete", "update"]
318321
- apiGroups: ["scheduling.incubator.k8s.io", "scheduling.sigs.dev"]
319322
resources: ["podgroups", "queues", "queues/status"]
320323
verbs: ["get", "list", "watch", "create", "delete", "update"]

‎pkg/apis/helpers/helpers.go‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,24 @@ func CreateConfigMapIfNotExist(job *vcbatch.Job, kubeClients kubernetes.Interfac
122122
return nil
123123
}
124124

125+
// CreateSecret create secret
126+
func CreateSecret(job *vcbatch.Job, kubeClients kubernetes.Interface, data map[string][]byte, secretName string) error {
127+
secret := &v1.Secret{
128+
ObjectMeta: metav1.ObjectMeta{
129+
Name: secretName,
130+
Namespace: job.Namespace,
131+
OwnerReferences: []metav1.OwnerReference{
132+
*metav1.NewControllerRef(job, JobKind),
133+
},
134+
},
135+
Data: data,
136+
}
137+
138+
_, err := kubeClients.CoreV1().Secrets(job.Namespace).Create(secret)
139+
140+
return err
141+
}
142+
125143
// DeleteConfigmap deletes the config map resource
126144
func DeleteConfigmap(job *vcbatch.Job, kubeClients kubernetes.Interface, cmName string) error {
127145
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
145163
return nil
146164
}
147165

166+
// DeleteSecret delete secret
167+
func DeleteSecret(job *vcbatch.Job, kubeClients kubernetes.Interface, secretName string) error {
168+
err := kubeClients.CoreV1().Secrets(job.Namespace).Delete(secretName, nil)
169+
if err != nil && true == apierrors.IsNotFound(err) {
170+
return nil
171+
}
172+
173+
return err
174+
}
175+
148176
// GeneratePodgroupName generate podgroup name of normal pod
149177
func GeneratePodgroupName(pod *v1.Pod) string {
150178
pgName := vcbatch.PodgroupNamePrefix

‎pkg/controllers/job/job_controller_actions_test.go‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ func TestKillJobFunc(t *testing.T) {
4242
JobInfo *apis.JobInfo
4343
Services []v1.Service
4444
ConfigMaps []v1.ConfigMap
45+
Secrets []v1.Secret
4546
Pods map[string]*v1.Pod
4647
Plugins []string
4748
ExpextVal error
@@ -52,6 +53,7 @@ func TestKillJobFunc(t *testing.T) {
5253
ObjectMeta: metav1.ObjectMeta{
5354
Name: "job1",
5455
Namespace: namespace,
56+
UID: "e7f18111-1cec-11ea-b688-fa163ec79500",
5557
},
5658
},
5759
PodGroup: &schedulingv1alpha2.PodGroup{
@@ -80,10 +82,10 @@ func TestKillJobFunc(t *testing.T) {
8082
},
8183
},
8284
},
83-
ConfigMaps: []v1.ConfigMap{
85+
Secrets: []v1.Secret{
8486
{
8587
ObjectMeta: metav1.ObjectMeta{
86-
Name: "job1-ssh",
88+
Name: "job1-e7f18111-1cec-11ea-b688-fa163ec79500-ssh",
8789
Namespace: namespace,
8890
},
8991
},
@@ -110,10 +112,10 @@ func TestKillJobFunc(t *testing.T) {
110112
}
111113
}
112114

113-
for _, configMap := range testcase.ConfigMaps {
114-
_, err := fakeController.kubeClient.CoreV1().ConfigMaps(namespace).Create(&configMap)
115+
for _, secret := range testcase.Secrets {
116+
_, err := fakeController.kubeClient.CoreV1().Secrets(namespace).Create(&secret)
115117
if err != nil {
116-
t.Error("Error While Creating ConfigMaps")
118+
t.Error("Error While Creating Secret.")
117119
}
118120
}
119121

@@ -142,26 +144,26 @@ func TestKillJobFunc(t *testing.T) {
142144

143145
err = fakeController.killJob(testcase.JobInfo, testcase.PodRetainPhase, testcase.UpdateStatus)
144146
if err != nil {
145-
t.Errorf("Case %d (%s): expected: No Error, but got error %s", i, testcase.Name, err.Error())
147+
t.Errorf("Case %d (%s): expected: No Error, but got error %v.", i, testcase.Name, err)
146148
}
147149

148150
for _, plugin := range testcase.Plugins {
149151

150152
if plugin == "svc" {
151153
_, err = fakeController.kubeClient.CoreV1().Services(namespace).Get(testcase.Job.Name, metav1.GetOptions{})
152154
if err == nil {
153-
t.Errorf("Case %d (%s): expected: Service to be deleted, but not deleted because of error %s", i, testcase.Name, err.Error())
155+
t.Errorf("Case %d (%s): expected: Service to be deleted, but not deleted.", i, testcase.Name)
154156
}
155157
}
156158

157159
if plugin == "ssh" {
158-
_, err := fakeController.kubeClient.CoreV1().ConfigMaps(namespace).Get(fmt.Sprint(testcase.Job.Name, "-ssh"), metav1.GetOptions{})
160+
_, err := fakeController.kubeClient.CoreV1().ConfigMaps(namespace).Get(
161+
fmt.Sprintf("%s-%s-%s", testcase.Job.Name, testcase.Job.UID, "ssh"), metav1.GetOptions{})
159162
if err == nil {
160-
t.Errorf("Case %d (%s): expected: ConfigMap to be deleted, but not deleted because of error %s", i, testcase.Name, err.Error())
163+
t.Errorf("Case %d (%s): expected: Secret to be deleted, but not deleted.", i, testcase.Name)
161164
}
162165
}
163166
}
164-
165167
})
166168
}
167169
}

‎pkg/controllers/job/job_controller_plugins_test.go‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ func TestPluginOnPodCreate(t *testing.T) {
5555
ObjectMeta: metav1.ObjectMeta{
5656
Name: "Job1",
5757
Namespace: namespace,
58+
UID: "e7f18111-1cec-11ea-b688-fa163ec79500",
5859
},
5960
},
6061
Pod: buildPod(namespace, "pod1", v1.PodPending, nil),
@@ -66,6 +67,7 @@ func TestPluginOnPodCreate(t *testing.T) {
6667
Job: &batch.Job{
6768
ObjectMeta: metav1.ObjectMeta{
6869
Name: "Job1",
70+
UID: "e7f18111-1cec-11ea-b688-fa163ec79500",
6971
},
7072
},
7173
Pod: buildPod(namespace, "pod1", v1.PodPending, nil),
@@ -124,7 +126,7 @@ func TestPluginOnPodCreate(t *testing.T) {
124126
}
125127
exist := false
126128
for _, volume := range container.VolumeMounts {
127-
if volume.Name == fmt.Sprint(testcase.Job.Name, "-ssh") {
129+
if volume.Name == fmt.Sprintf("%s-%s-%s", testcase.Job.Name, testcase.Job.UID, "ssh") {
128130
exist = true
129131
}
130132
}
@@ -153,6 +155,7 @@ func TestPluginOnJobAdd(t *testing.T) {
153155
ObjectMeta: metav1.ObjectMeta{
154156
Name: "job1",
155157
Namespace: namespace,
158+
UID: "e7f18111-1cec-11ea-b688-fa163ec79500",
156159
},
157160
},
158161
Plugins: []string{"svc", "ssh", "env"},
@@ -163,6 +166,7 @@ func TestPluginOnJobAdd(t *testing.T) {
163166
Job: &batch.Job{
164167
ObjectMeta: metav1.ObjectMeta{
165168
Name: "Job1",
169+
UID: "e7f18111-1cec-11ea-b688-fa163ec79500",
166170
},
167171
},
168172
Plugins: []string{"new"},
@@ -202,9 +206,10 @@ func TestPluginOnJobAdd(t *testing.T) {
202206
}
203207

204208
if plugin == "ssh" {
205-
_, err := fakeController.kubeClient.CoreV1().ConfigMaps(namespace).Get(fmt.Sprint(testcase.Job.Name, "-ssh"), metav1.GetOptions{})
209+
_, err := fakeController.kubeClient.CoreV1().Secrets(namespace).Get(
210+
fmt.Sprintf("%s-%s-%s", testcase.Job.Name, testcase.Job.UID, "ssh"), metav1.GetOptions{})
206211
if err != nil {
207-
t.Errorf("Case %d (%s): expected: ConfigMap to be created, but not created because of error %s", i, testcase.Name, err.Error())
212+
t.Errorf("Case %d (%s): expected: Secret to be created, but not created because of error %s", i, testcase.Name, err.Error())
208213
}
209214
}
210215

@@ -233,6 +238,7 @@ func TestPluginOnJobDelete(t *testing.T) {
233238
ObjectMeta: metav1.ObjectMeta{
234239
Name: "job1",
235240
Namespace: namespace,
241+
UID: "e7f18111-1cec-11ea-b688-fa163ec79500",
236242
},
237243
},
238244
Plugins: []string{"svc", "ssh", "env"},
@@ -243,6 +249,7 @@ func TestPluginOnJobDelete(t *testing.T) {
243249
Job: &batch.Job{
244250
ObjectMeta: metav1.ObjectMeta{
245251
Name: "Job1",
252+
UID: "e7f18111-1cec-11ea-b688-fa163ec79500",
246253
},
247254
},
248255
Plugins: []string{"new"},
@@ -272,23 +279,23 @@ func TestPluginOnJobDelete(t *testing.T) {
272279
if plugin == "svc" {
273280
_, err := fakeController.kubeClient.CoreV1().ConfigMaps(namespace).Get(fmt.Sprint(testcase.Job.Name, "-svc"), metav1.GetOptions{})
274281
if err == nil {
275-
t.Errorf("Case %d (%s): expected: ConfigMap to be deleted, but not deleted because of error %s", i, testcase.Name, err.Error())
282+
t.Errorf("Case %d (%s): expected: ConfigMap to be deleted, but not deleted.", i, testcase.Name)
276283
}
277284

278285
_, err = fakeController.kubeClient.CoreV1().Services(namespace).Get(testcase.Job.Name, metav1.GetOptions{})
279286
if err == nil {
280-
t.Errorf("Case %d (%s): expected: Service to be deleted, but not deleted because of error %s", i, testcase.Name, err.Error())
287+
t.Errorf("Case %d (%s): expected: Service to be deleted, but not deleted.", i, testcase.Name)
281288
}
282289
}
283290

284291
if plugin == "ssh" {
285-
_, err := fakeController.kubeClient.CoreV1().ConfigMaps(namespace).Get(fmt.Sprint(testcase.Job.Name, "-ssh"), metav1.GetOptions{})
292+
_, err := fakeController.kubeClient.CoreV1().Secrets(namespace).Get(
293+
fmt.Sprintf("%s-%s-%s", testcase.Job.Name, testcase.Job.UID, "ssh"), metav1.GetOptions{})
286294
if err == nil {
287-
t.Errorf("Case %d (%s): expected: ConfigMap to be deleted, but not deleted because of error %s", i, testcase.Name, err.Error())
295+
t.Errorf("Case %d (%s): expected: secret to be deleted, but not deleted.", i, testcase.Name)
288296
}
289297
}
290298
}
291299
})
292-
293300
}
294301
}

‎pkg/controllers/job/plugins/ssh/ssh.go‎

Lines changed: 18 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -84,8 +84,9 @@ func (sp *sshPlugin) OnJobAdd(job *batch.Job) error {
8484
return err
8585
}
8686

87-
if err := helpers.CreateConfigMapIfNotExist(job, sp.Clientset.KubeClients, data, sp.cmName(job)); err != nil {
88-
return err
87+
if err := helpers.CreateSecret(job, sp.Clientset.KubeClients, data, sp.secretName(job)); err != nil {
88+
return fmt.Errorf("create secret for job <%s/%s> with ssh plugin failed for %v",
89+
job.Namespace, job.Name, err)
8990
}
9091

9192
job.Status.ControlledResources["plugin-"+sp.Name()] = sp.Name()
@@ -94,24 +95,19 @@ func (sp *sshPlugin) OnJobAdd(job *batch.Job) error {
9495
}
9596

9697
func (sp *sshPlugin) OnJobDelete(job *batch.Job) error {
97-
if err := helpers.DeleteConfigmap(job, sp.Clientset.KubeClients, sp.cmName(job)); err != nil {
98-
return err
99-
}
100-
101-
return nil
98+
return helpers.DeleteSecret(job, sp.Clientset.KubeClients, sp.secretName(job))
10299
}
103100

104101
func (sp *sshPlugin) mountRsaKey(pod *v1.Pod, job *batch.Job) {
102+
secretName := sp.secretName(job)
105103

106-
cmName := sp.cmName(job)
107104
sshVolume := v1.Volume{
108-
Name: cmName,
105+
Name: secretName,
109106
}
107+
110108
var mode int32 = 0600
111-
sshVolume.ConfigMap = &v1.ConfigMapVolumeSource{
112-
LocalObjectReference: v1.LocalObjectReference{
113-
Name: cmName,
114-
},
109+
sshVolume.Secret = &v1.SecretVolumeSource{
110+
SecretName: secretName,
115111
Items: []v1.KeyToPath{
116112
{
117113
Key: SSHPrivateKey,
@@ -135,7 +131,7 @@ func (sp *sshPlugin) mountRsaKey(pod *v1.Pod, job *batch.Job) {
135131

136132
if sp.sshKeyFilePath != SSHAbsolutePath {
137133
var noRootMode int32 = 0755
138-
sshVolume.ConfigMap.DefaultMode = &noRootMode
134+
sshVolume.Secret.DefaultMode = &noRootMode
139135
}
140136

141137
pod.Spec.Volumes = append(pod.Spec.Volumes, sshVolume)
@@ -144,7 +140,7 @@ func (sp *sshPlugin) mountRsaKey(pod *v1.Pod, job *batch.Job) {
144140
vm := v1.VolumeMount{
145141
MountPath: sp.sshKeyFilePath,
146142
SubPath: SSHRelativePath,
147-
Name: cmName,
143+
Name: secretName,
148144
}
149145

150146
pod.Spec.Containers[i].VolumeMounts = append(c.VolumeMounts, vm)
@@ -153,7 +149,7 @@ func (sp *sshPlugin) mountRsaKey(pod *v1.Pod, job *batch.Job) {
153149
return
154150
}
155151

156-
func generateRsaKey(job *batch.Job) (map[string]string, error) {
152+
func generateRsaKey(job *batch.Job) (map[string][]byte, error) {
157153
bitSize := 1024
158154

159155
privateKey, err := rsa.GenerateKey(rand.Reader, bitSize)
@@ -177,16 +173,16 @@ func generateRsaKey(job *batch.Job) (map[string]string, error) {
177173
}
178174
publicKeyBytes := ssh.MarshalAuthorizedKey(publicRsaKey)
179175

180-
data := make(map[string]string)
181-
data[SSHPrivateKey] = string(privateKeyBytes)
182-
data[SSHPublicKey] = string(publicKeyBytes)
183-
data[SSHConfig] = generateSSHConfig(job)
176+
data := make(map[string][]byte)
177+
data[SSHPrivateKey] = privateKeyBytes
178+
data[SSHPublicKey] = publicKeyBytes
179+
data[SSHConfig] = []byte(generateSSHConfig(job))
184180

185181
return data, nil
186182
}
187183

188-
func (sp *sshPlugin) cmName(job *batch.Job) string {
189-
return fmt.Sprintf("%s-%s", job.Name, sp.Name())
184+
func (sp *sshPlugin) secretName(job *batch.Job) string {
185+
return fmt.Sprintf("%s-%s-%s", job.Name, job.UID, sp.Name())
190186
}
191187

192188
func (sp *sshPlugin) addFlags() {

‎test/e2e/job_plugins.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -148,8 +148,8 @@ var _ = Describe("Job E2E Test: Test Job Plugins", func() {
148148
err := waitJobReady(context, job)
149149
Expect(err).NotTo(HaveOccurred())
150150

151-
pluginName := fmt.Sprintf("%s-ssh", jobName)
152-
_, err = context.kubeclient.CoreV1().ConfigMaps(namespace).Get(
151+
pluginName := fmt.Sprintf("%s-%s-ssh", jobName, job.UID)
152+
_, err = context.kubeclient.CoreV1().Secrets(namespace).Get(
153153
pluginName, v1.GetOptions{})
154154
Expect(err).NotTo(HaveOccurred())
155155

@@ -206,8 +206,8 @@ var _ = Describe("Job E2E Test: Test Job Plugins", func() {
206206
err := waitJobReady(context, job)
207207
Expect(err).NotTo(HaveOccurred())
208208

209-
pluginName := fmt.Sprintf("%s-ssh", jobName)
210-
_, err = context.kubeclient.CoreV1().ConfigMaps(namespace).Get(
209+
pluginName := fmt.Sprintf("%s-%s-ssh", jobName, job.UID)
210+
_, err = context.kubeclient.CoreV1().Secrets(namespace).Get(
211211
pluginName, v1.GetOptions{})
212212
Expect(err).NotTo(HaveOccurred())
213213

0 commit comments

Comments
 (0)