This is an automated email from the ASF dual-hosted git repository.
roryqi pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-uniffle.git
The following commit(s) were added to refs/heads/master by this push:
new 87361da6 [Bug] Fix invalid owner of host path volumes (#323) (#330)
87361da6 is described below
commit 87361da66a467fabbdbecf25061b8e2782c5d9ce
Author: jasonawang <[email protected]>
AuthorDate: Wed Nov 16 16:40:43 2022 +0800
[Bug] Fix invalid owner of host path volumes (#323) (#330)
### What changes were proposed in this pull request?
fix #323
I have modified it for time reasons, thanks for pointing this out.
I changed the owner of host paths as UID:GID.
### Why are the changes needed?
Fix bug
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
UT
---
.../pkg/controller/controller/process_rss_test.go | 44 +++++-----
.../operator/pkg/controller/util/util.go | 14 ++-
.../operator/pkg/controller/util/util_test.go | 99 ++++++++++++++++++++++
deploy/kubernetes/operator/pkg/utils/util.go | 15 ----
4 files changed, 134 insertions(+), 38 deletions(-)
diff --git
a/deploy/kubernetes/operator/pkg/controller/controller/process_rss_test.go
b/deploy/kubernetes/operator/pkg/controller/controller/process_rss_test.go
index a867bb62..a5ac8748 100644
--- a/deploy/kubernetes/operator/pkg/controller/controller/process_rss_test.go
+++ b/deploy/kubernetes/operator/pkg/controller/controller/process_rss_test.go
@@ -76,26 +76,28 @@ func TestProcessEmptyPhaseRss(t *testing.T) {
expectedNeedRetry: false,
},
} {
- needRetry, err := rc.processNormal(rss)
- if err != nil {
- t.Errorf("process rss object failed: %v", err)
- return
- }
- if needRetry != tt.expectedNeedRetry {
- t.Errorf("unexpected result indicates whether to
retrys: %v, expected: %v",
- needRetry, tt.expectedNeedRetry)
- return
- }
- updatedRss, getErr :=
rssClient.UniffleV1alpha1().RemoteShuffleServices(rss.Namespace).
- Get(context.TODO(), rss.Name, metav1.GetOptions{})
- if getErr != nil {
- t.Errorf("get updated rss object failed: %v", err)
- return
- }
- if !reflect.DeepEqual(updatedRss.Status, tt.expectedRssStatus) {
- t.Errorf("unexpected status of updated rss object: %+v,
expected: %+v",
- updatedRss.Status, tt.expectedRssStatus)
- return
- }
+ t.Run(tt.name, func(tc *testing.T) {
+ needRetry, err := rc.processNormal(rss)
+ if err != nil {
+ tc.Errorf("process rss object failed: %v", err)
+ return
+ }
+ if needRetry != tt.expectedNeedRetry {
+ tc.Errorf("unexpected result indicates whether
to retrys: %v, expected: %v",
+ needRetry, tt.expectedNeedRetry)
+ return
+ }
+ updatedRss, getErr :=
rssClient.UniffleV1alpha1().RemoteShuffleServices(rss.Namespace).
+ Get(context.TODO(), rss.Name,
metav1.GetOptions{})
+ if getErr != nil {
+ tc.Errorf("get updated rss object failed: %v",
err)
+ return
+ }
+ if !reflect.DeepEqual(updatedRss.Status,
tt.expectedRssStatus) {
+ tc.Errorf("unexpected status of updated rss
object: %+v, expected: %+v",
+ updatedRss.Status, tt.expectedRssStatus)
+ return
+ }
+ })
}
}
diff --git a/deploy/kubernetes/operator/pkg/controller/util/util.go
b/deploy/kubernetes/operator/pkg/controller/util/util.go
index 6f588681..bc1f3ccb 100644
--- a/deploy/kubernetes/operator/pkg/controller/util/util.go
+++ b/deploy/kubernetes/operator/pkg/controller/util/util.go
@@ -209,9 +209,19 @@ func generateLogVolumeMount(rssPodSpec
*v1alpha1.RSSPodSpec) *corev1.VolumeMount
func generateMakeDataDirCommand(rssPodSpec *v1alpha1.RSSPodSpec) []string {
var commands []string
- fsGroup := *rssPodSpec.SecurityContext.FSGroup
+
+ var runAsUser int64
+ if rssPodSpec.SecurityContext != nil &&
rssPodSpec.SecurityContext.RunAsUser != nil {
+ runAsUser = *rssPodSpec.SecurityContext.RunAsUser
+ }
+
+ var fsGroup int64
+ if rssPodSpec.SecurityContext != nil &&
rssPodSpec.SecurityContext.FSGroup != nil {
+ fsGroup = *rssPodSpec.SecurityContext.FSGroup
+ }
+
for _, mountPath := range rssPodSpec.HostPathMounts {
- commands = append(commands, fmt.Sprintf("chown -R %v:%v %v",
fsGroup, fsGroup, mountPath))
+ commands = append(commands, fmt.Sprintf("chown -R %v:%v %v",
runAsUser, fsGroup, mountPath))
}
return commands
}
diff --git a/deploy/kubernetes/operator/pkg/controller/util/util_test.go
b/deploy/kubernetes/operator/pkg/controller/util/util_test.go
new file mode 100644
index 00000000..5a49c5ef
--- /dev/null
+++ b/deploy/kubernetes/operator/pkg/controller/util/util_test.go
@@ -0,0 +1,99 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package util
+
+import (
+ "sort"
+ "testing"
+
+ corev1 "k8s.io/api/core/v1"
+ "k8s.io/utils/pointer"
+
+
"github.com/apache/incubator-uniffle/deploy/kubernetes/operator/api/uniffle/v1alpha1"
+)
+
+func TestGenerateMakeDataDirCommand(t *testing.T) {
+ for _, tt := range []struct {
+ name string
+ rssPodSpec *v1alpha1.RSSPodSpec
+ expectedCommands []string
+ }{
+ {
+ name: "empty security context",
+ rssPodSpec: &v1alpha1.RSSPodSpec{
+ HostPathMounts: map[string]string{
+ "/data1": "/mnt/data1",
+ },
+ },
+ expectedCommands: []string{
+ "chown -R 0:0 /mnt/data1",
+ },
+ },
+ {
+ name: "empty runAsUser field in security context",
+ rssPodSpec: &v1alpha1.RSSPodSpec{
+ HostPathMounts: map[string]string{
+ "/data2": "/mnt/data2",
+ },
+ SecurityContext: &corev1.PodSecurityContext{
+ FSGroup: pointer.Int64(1000),
+ },
+ },
+ expectedCommands: []string{
+ "chown -R 0:1000 /mnt/data2",
+ },
+ },
+ {
+ name: "non empty field of runAsUser and fsGroup in
security context",
+ rssPodSpec: &v1alpha1.RSSPodSpec{
+ HostPathMounts: map[string]string{
+ "/data3": "/mnt/data3",
+ },
+ SecurityContext: &corev1.PodSecurityContext{
+ RunAsUser: pointer.Int64(2000),
+ FSGroup: pointer.Int64(1000),
+ },
+ },
+ expectedCommands: []string{
+ "chown -R 2000:1000 /mnt/data3",
+ },
+ },
+ } {
+ t.Run(tt.name, func(tc *testing.T) {
+ commands := generateMakeDataDirCommand(tt.rssPodSpec)
+ if !isEqualStringSlice(commands, tt.expectedCommands) {
+ tc.Errorf("unexpected commands: %+v, expected:
%+v", commands, tt.expectedCommands)
+ return
+ }
+ })
+ }
+}
+
+func isEqualStringSlice(a, b []string) bool {
+ if len(a) != len(b) {
+ return false
+ }
+ sort.Strings(a)
+ sort.Strings(b)
+ for i := range a {
+ if a[i] != b[i] {
+ return false
+ }
+ }
+ return true
+}
diff --git a/deploy/kubernetes/operator/pkg/utils/util.go
b/deploy/kubernetes/operator/pkg/utils/util.go
index bd336f48..4c609441 100644
--- a/deploy/kubernetes/operator/pkg/utils/util.go
+++ b/deploy/kubernetes/operator/pkg/utils/util.go
@@ -26,7 +26,6 @@ import (
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/sets"
- "k8s.io/client-go/informers"
"k8s.io/client-go/kubernetes"
"k8s.io/client-go/util/retry"
"k8s.io/klog/v2"
@@ -35,20 +34,6 @@ import (
"github.com/apache/incubator-uniffle/deploy/kubernetes/operator/pkg/constants"
)
-// CreatePodInformerFactory creates pod informer factory by label selector.
-func CreatePodInformerFactory(kubeClient kubernetes.Interface,
- key, value string) informers.SharedInformerFactory {
- option := func(options *metav1.ListOptions) {
- if len(value) > 0 {
- options.LabelSelector = key + "=" + value
- } else {
- options.LabelSelector = key
- }
- }
- return informers.NewSharedInformerFactoryWithOptions(kubeClient, 0,
- informers.WithTweakListOptions(option))
-}
-
// GetCurrentNamespace returns current namespace.
func GetCurrentNamespace() string {
namespace := os.Getenv(constants.PodNamespaceEnv)
