Repository: FoundationDB/fdb-kubernetes-operator
Pull request #1522 (2023-02-27 02:18:39)

Complement ProcessRole and ProcessClass
# Description

ProcessRole and ProcessClass, the two types didn't include all the existing roles and classes, this PR complements them.

## Type of change

- Bug fix (non-breaking change which fixes an issue)

## Discussion

No.

## Testing

No.

## Documentation

No, didn't need to update any doc.

## Follow-up

No.


foundationdb-ci: 
### Result of fdb-kubernetes-operator-pr on Linux CentOS 7

* Commit ID: 9ce957951c468ce1985e7c54534c28e54e949c1b
* Duration 1:50:52
* Result: :white_check_mark: **SUCCEEDED**
* Error: `N/A`
* [Build Log](https://d1e0l78xbkh3xa.cloudfront.net/fdb-kubernetes-operator/fdb-kubernetes-operator-pr/c9d506d1-a584-4b7f-8dff-3bb6ee8d411d.gz) terminal output (available for 30 days)
* [Build Workspace](https://d1e0l78xbkh3xa.cloudfront.net/fdb-kubernetes-operator-pr/c9d506d1-a584-4b7f-8dff-3bb6ee8d411d/workspace.zip) zip file of the working directory (available for 30 days)


Patch:
diff --git a/api/v1beta2/foundationdb_process_class.go b/api/v1beta2/foundationdb_process_class.go
index d17bafa9..98f2789b 100644
--- a/api/v1beta2/foundationdb_process_class.go
+++ b/api/v1beta2/foundationdb_process_class.go
@@ -40,6 +40,12 @@ const (
 	ProcessClassTest ProcessClass = "test"
 	// ProcessClassCoordinator model for FDB class coordinator
 	ProcessClassCoordinator ProcessClass = "coordinator"
+	// ProcessClassProxy model for FDB proxy processes
+	ProcessClassProxy ProcessClass = "proxy"
+	// ProcessClassCommitProxy model for FDB commit_proxy processes
+	ProcessClassCommitProxy ProcessClass = "commit_proxy"
+	// ProcessClassGrvProxy model for FDB grv_proxy processes
+	ProcessClassGrvProxy ProcessClass = "grv_proxy"
 )
 
 // IsStateful determines whether a process class should store data.
diff --git a/api/v1beta2/foundationdb_status.go b/api/v1beta2/foundationdb_status.go
index 1e62d3b9..ab1ad2dd 100644
--- a/api/v1beta2/foundationdb_status.go
+++ b/api/v1beta2/foundationdb_status.go
@@ -296,10 +296,31 @@ type FoundationDBStatusBackupTag struct {
 // ProcessRole models the role of a pod.
 type ProcessRole string
 
-// TODO (johscheuer): add more roles to this list
 const (
 	// ProcessRoleCoordinator model for FDB coordinator role.
 	ProcessRoleCoordinator ProcessRole = "coordinator"
+	// ProcessRoleClusterController model for FDB cluster_controller role
+	ProcessRoleClusterController ProcessRole = "cluster_controller"
+	// ProcessRoleStorage model for FDB storage role
+	ProcessRoleStorage ProcessRole = "storage"
+	// ProcessRoleLog model for FDB log role
+	ProcessRoleLog ProcessRole = "log"
+	// ProcessRoleSequencer model for FDB sequencer role
+	ProcessRoleSequencer ProcessRole = "sequencer"
+	// ProcessRoleMaster model for FDB master role
+	ProcessRoleMaster ProcessRole = "master"
+	// ProcessRoleProxy model for FDB proxy role
+	ProcessRoleProxy ProcessRole = "proxy"
+	// ProcessRoleGrvProxy model for FDB grv_proxy role
+	ProcessRoleGrvProxy ProcessRole = "grv_proxy"
+	// ProcessRoleCommitProxy model for FDB commit_proxy role
+	ProcessRoleCommitProxy ProcessRole = "commit_proxy"
+	// ProcessRoleResolver model for FDB resolver role
+	ProcessRoleResolver ProcessRole = "resolver"
+	// ProcessRoleDataDistributor model for FDB data_distributor role
+	ProcessRoleDataDistributor ProcessRole = "data_distributor"
+	// ProcessRoleRatekeeper model for FDB ratekeeper role
+	ProcessRoleRatekeeper ProcessRole = "ratekeeper"
 )
 
 // RecoveryState represents the recovery state from the FDB cluster json.
diff --git a/api/v1beta2/foundationdbcluster_types.go b/api/v1beta2/foundationdbcluster_types.go
index 65b14f71..7773db16 100644
--- a/api/v1beta2/foundationdbcluster_types.go
+++ b/api/v1beta2/foundationdbcluster_types.go
@@ -1717,9 +1717,13 @@ type RequiredAddressSet struct {
 // CrashLoopContainerObject specifies crash-loop target for specific container.
 type CrashLoopContainerObject struct {
 	// Name of the target container.
+	// +kubebuilder:validation:MinLength=1
+	// +kubebuilder:validation:MaxLength=253
 	ContainerName string `json:"containerName,omitempty"`
 
 	// Target processes to kill inside the container.
+	// +kubebuilder:validation:MinItems=0
+	// +kubebuilder:validation:MaxItems=10000
 	Targets []ProcessGroupID `json:"targets,omitempty"`
 }
 
@@ -1735,6 +1739,8 @@ type BuggifyConfig struct {
 
 	// CrashLoopContainers defines a list of process group IDs and containers
 	// that should be put into a crash looping state.
+	// +kubebuilder:validation:MinItems=0
+	// +kubebuilder:validation:MaxItems=8
 	CrashLoopContainers []CrashLoopContainerObject `json:"crashLoopContainers,omitempty"`
 
 	// EmptyMonitorConf instructs the operator to update all of the fdbmonitor.conf
diff --git a/config/crd/bases/apps.foundationdb.org_foundationdbclusters.yaml b/config/crd/bases/apps.foundationdb.org_foundationdbclusters.yaml
index feb5b140..3ee650cc 100644
--- a/config/crd/bases/apps.foundationdb.org_foundationdbclusters.yaml
+++ b/config/crd/bases/apps.foundationdb.org_foundationdbclusters.yaml
@@ -9940,13 +9940,19 @@ spec:
                     items:
                       properties:
                         containerName:
+                          maxLength: 253
+                          minLength: 1
                           type: string
                         targets:
                           items:
                             maxLength: 63
                             type: string
+                          maxItems: 10000
+                          minItems: 0
                           type: array
                       type: object
+                    maxItems: 8
+                    minItems: 0
                     type: array
                   emptyMonitorConf:
                     type: boolean
diff --git a/setup/setup.go b/setup/setup.go
index 1125250d..7d9577f8 100644
--- a/setup/setup.go
+++ b/setup/setup.go
@@ -24,8 +24,11 @@ import (
 	"flag"
 	"fmt"
 	"io"
+	"io/fs"
+	"log"
 	"os"
 	"path"
+	"strconv"
 	"strings"
 	"time"
 
@@ -59,6 +62,7 @@ type Options struct {
 	MetricsAddr                        string
 	LeaderElectionID                   string
 	LogFile                            string
+	LogFilePermission                  string
 	LabelSelector                      string
 	WatchNamespace                     string
 	CliTimeout                         int
@@ -89,6 +93,8 @@ func (o *Options) BindFlags(fs *flag.FlagSet) {
 	fs.DurationVar(&o.LogFileMinAge, "log-file-min-age", 5*time.Minute, "Defines the minimum age of fdbcli log files before removing when \"--cleanup-old-cli-logs\" is set.")
 	fs.IntVar(&o.LogFileMaxAge, "log-file-max-age", 28, "Defines the maximum age to retain old operator log file in number of days.")
 	fs.IntVar(&o.LogFileMaxSize, "log-file-max-size", 250, "Defines the maximum size in megabytes of the operator log file before it gets rotated.")
+	fs.StringVar(&o.LogFilePermission, "log-file-permission", "0644",
+		"The file permission for the log file. Only used if log-file is set. Only the octal representation is supported.")
 	fs.IntVar(&o.MaxNumberOfOldLogFiles, "max-old-log-files", 3, "Defines the maximum number of old operator log files to retain.")
 	fs.BoolVar(&o.CompressOldFiles, "compress", false, "Defines whether the rotated log files should be compressed using gzip or not.")
 	fs.BoolVar(&o.PrintVersion, "version", false, "Prints the version of the operator and exits.")
@@ -113,25 +119,14 @@ func StartManager(
 	restoreReconciler *controllers.FoundationDBRestoreReconciler,
 	logr logr.Logger,
 	watchedObjects ...client.Object) (manager.Manager, *os.File) {
-	var logWriter io.Writer
-	var file *os.File
-
 	if operatorOpts.PrintVersion {
 		fmt.Printf("version: %s\n", operatorVersion)
 		os.Exit(0)
 	}
 
-	if operatorOpts.LogFile != "" {
-		lumberjackLogger := &lumberjack.Logger{
-			Filename:   operatorOpts.LogFile,
-			MaxSize:    operatorOpts.LogFileMaxSize,
-			MaxAge:     operatorOpts.LogFileMaxAge,
-			MaxBackups: operatorOpts.MaxNumberOfOldLogFiles,
-			Compress:   operatorOpts.CompressOldFiles,
-		}
-		logWriter = io.MultiWriter(os.Stdout, lumberjackLogger)
-	} else {
-		logWriter = os.Stdout
+	logWriter, err := setupLogger(operatorOpts)
+	if err != nil {
+		log.Fatalf("unable to setup logger: %s, got error: %s\n", operatorOpts.LogFile, err.Error())
 	}
 
 	logger := zap.New(
@@ -239,7 +234,7 @@ func StartManager(
 
 	// +kubebuilder:scaffold:builder
 	setupLog.Info("setup manager")
-	return mgr, file
+	return mgr, nil
 }
 
 // MoveFDBBinaries moves FDB binaries that are pulled from setup containers into
@@ -322,3 +317,51 @@ func moveFDBBinaries(log logr.Logger) error {
 
 	return nil
 }
+
+// setupLogger will return a MultiWriter if the operator should log to a file and stdout otherwise only the stdout
+// io.Writer is returned. If the operator should log to a file the operator will make sure to create the file with
+// the expected permissions.
+func setupLogger(operatorOpts Options) (io.Writer, error) {
+	if operatorOpts.LogFile != "" {
+		expectedPermission := fs.FileMode(0644)
+
+		if operatorOpts.LogFilePermission != "" {
+			expectedPermissionUnit, err := strconv.ParseUint(operatorOpts.LogFilePermission, 8, 32)
+			if err != nil {
+				return nil, err
+			}
+
+			expectedPermission = fs.FileMode(expectedPermissionUnit)
+		}
+
+		// We have to create the original file by ourself since lumberjack doesn't support to pass down the expected permissions
+		// see: https://github.com/natefinch/lumberjack/issues/82#issuecomment-482143273.
+		stat, err := os.Stat(operatorOpts.LogFile)
+		// File doesn't exist and must be created with the expected permission.
+		if os.IsNotExist(err) {
+			err := os.WriteFile(operatorOpts.LogFile, nil, expectedPermission)
+			if err != nil {
+				return nil, err
+			}
+		}
+
+		if err == nil && stat.Mode() != expectedPermission {
+			err = os.Chmod(operatorOpts.LogFile, expectedPermission)
+			if err != nil {
+				return nil, err
+			}
+		}
+
+		lumberjackLogger := &lumberjack.Logger{
+			Filename:   operatorOpts.LogFile,
+			MaxSize:    operatorOpts.LogFileMaxSize,
+			MaxAge:     operatorOpts.LogFileMaxAge,
+			MaxBackups: operatorOpts.MaxNumberOfOldLogFiles,
+			Compress:   operatorOpts.CompressOldFiles,
+		}
+
+		return io.MultiWriter(os.Stdout, lumberjackLogger), nil
+	}
+
+	return os.Stdout, nil
+}


Test patch:
diff --git a/api/v1beta2/foundationdb_status_test.go b/api/v1beta2/foundationdb_status_test.go
index 8a7c5f4c..e32c2703 100644
--- a/api/v1beta2/foundationdb_status_test.go
+++ b/api/v1beta2/foundationdb_status_test.go
@@ -104,7 +104,7 @@ var _ = Describe("FoundationDBStatus", func() {
 							UptimeSeconds: 2955.58,
 							Roles: []FoundationDBStatusProcessRoleInfo{
 								{
-									Role: "log",
+									Role: string(ProcessRoleLog),
 								},
 							},
 							Messages: []FoundationDBStatusProcessMessage{},
@@ -127,10 +127,10 @@ var _ = Describe("FoundationDBStatus", func() {
 							UptimeSeconds: 2475.33,
 							Roles: []FoundationDBStatusProcessRoleInfo{
 								{
-									Role: "proxy",
+									Role: string(ProcessRoleProxy),
 								},
 								{
-									Role: "storage",
+									Role: string(ProcessRoleStorage),
 								},
 							},
 							Messages: []FoundationDBStatusProcessMessage{},
@@ -153,10 +153,10 @@ var _ = Describe("FoundationDBStatus", func() {
 							UptimeSeconds: 2951.17,
 							Roles: []FoundationDBStatusProcessRoleInfo{
 								{
-									Role: "proxy",
+									Role: string(ProcessRoleProxy),
 								},
 								{
-									Role: "storage",
+									Role: string(ProcessRoleStorage),
 								},
 							},
 							Messages: []FoundationDBStatusProcessMessage{},
@@ -179,10 +179,10 @@ var _ = Describe("FoundationDBStatus", func() {
 							UptimeSeconds: 710.119,
 							Roles: []FoundationDBStatusProcessRoleInfo{
 								{
-									Role: "cluster_controller",
+									Role: string(ProcessRoleClusterController),
 								},
 								{
-									Role: "log",
+									Role: string(ProcessRoleLog),
 								},
 							},
 							Messages: []FoundationDBStatusProcessMessage{},
@@ -208,7 +208,7 @@ var _ = Describe("FoundationDBStatus", func() {
 									Role: string(ProcessRoleCoordinator),
 								},
 								{
-									Role: "resolver",
+									Role: string(ProcessRoleResolver),
 								},
 							},
 							Messages: []FoundationDBStatusProcessMessage{},
@@ -231,19 +231,19 @@ var _ = Describe("FoundationDBStatus", func() {
 							UptimeSeconds: 880.18,
 							Roles: []FoundationDBStatusProcessRoleInfo{
 								{
-									Role: "master",
+									Role: string(ProcessRoleMaster),
 								},
 								{
-									Role: "data_distributor",
+									Role: string(ProcessRoleDataDistributor),
 								},
 								{
-									Role: "ratekeeper",
+									Role: string(ProcessRoleRatekeeper),
 								},
 								{
 									Role: string(ProcessRoleCoordinator),
 								},
 								{
-									Role: "log",
+									Role: string(ProcessRoleLog),
 								},
 							},
 							Messages: []FoundationDBStatusProcessMessage{},
@@ -269,10 +269,10 @@ var _ = Describe("FoundationDBStatus", func() {
 									Role: string(ProcessRoleCoordinator),
 								},
 								{
-									Role: "proxy",
+									Role: string(ProcessRoleProxy),
 								},
 								{
-									Role: "storage",
+									Role: string(ProcessRoleStorage),
 								},
 							},
 							Messages: []FoundationDBStatusProcessMessage{},
@@ -500,9 +500,9 @@ var _ = Describe("FoundationDBStatus", func() {
 					Version:       "7.1.0-rc1",
 					UptimeSeconds: 85.0026,
 					Roles: []FoundationDBStatusProcessRoleInfo{
-						{Role: "coordinator"},
-						{Role: "grv_proxy"},
-						{Role: "storage"},
+						{Role: string(ProcessRoleCoordinator)},
+						{Role: string(ProcessRoleGrvProxy)},
+						{Role: string(ProcessRoleStorage)},
 					},
 					Messages: []FoundationDBStatusProcessMessage{},
 				},
@@ -523,9 +523,9 @@ var _ = Describe("FoundationDBStatus", func() {
 					Version:       "7.1.0-rc1",
 					UptimeSeconds: 85.0031,
 					Roles: []FoundationDBStatusProcessRoleInfo{
-						{Role: "coordinator"},
+						{Role: string(ProcessRoleCoordinator)},
 						{Role: string(ProcessClassStorage)},
-						{Role: "resolver"},
+						{Role: string(ProcessRoleResolver)},
 					},
 					Messages: []FoundationDBStatusProcessMessage{},
 				},
@@ -546,9 +546,9 @@ var _ = Describe("FoundationDBStatus", func() {
 					Version:       "7.1.0-rc1",
 					UptimeSeconds: 85.0029,
 					Roles: []FoundationDBStatusProcessRoleInfo{
-						{Role: "coordinator"},
-						{Role: "commit_proxy"},
-						{Role: "storage"},
+						{Role: string(ProcessRoleCoordinator)},
+						{Role: string(ProcessRoleCommitProxy)},
+						{Role: string(ProcessRoleStorage)},
 					},
 					Messages: []FoundationDBStatusProcessMessage{},
 				},
@@ -569,9 +569,9 @@ var _ = Describe("FoundationDBStatus", func() {
 					Version:       "7.1.0-rc1",
 					UptimeSeconds: 85.0027,
 					Roles: []FoundationDBStatusProcessRoleInfo{
-						{Role: "master"},
-						{Role: "data_distributor"},
-						{Role: "ratekeeper"},
+						{Role: string(ProcessRoleMaster)},
+						{Role: string(ProcessRoleDataDistributor)},
+						{Role: string(ProcessRoleRatekeeper)},
 					},
 					Messages: []FoundationDBStatusProcessMessage{},
 				},
@@ -613,7 +613,7 @@ var _ = Describe("FoundationDBStatus", func() {
 					Version:       "7.1.0-rc1",
 					UptimeSeconds: 85.0029,
 					Roles: []FoundationDBStatusProcessRoleInfo{
-						{Role: "log"},
+						{Role: string(ProcessRoleLog)},
 					},
 					Messages: []FoundationDBStatusProcessMessage{},
 				},
@@ -634,7 +634,7 @@ var _ = Describe("FoundationDBStatus", func() {
 					Version:       "7.1.0-rc1",
 					UptimeSeconds: 85.003,
 					Roles: []FoundationDBStatusProcessRoleInfo{
-						{Role: "log"},
+						{Role: string(ProcessRoleLog)},
 					},
 					Messages: []FoundationDBStatusProcessMessage{},
 				},
@@ -655,7 +655,7 @@ var _ = Describe("FoundationDBStatus", func() {
 					Version:       "7.1.0-rc1",
 					UptimeSeconds: 85.0027,
 					Roles: []FoundationDBStatusProcessRoleInfo{
-						{Role: "log"},
+						{Role: string(ProcessRoleLog)},
 					},
 					Messages: []FoundationDBStatusProcessMessage{},
 				},
diff --git a/setup/setup_test.go b/setup/setup_test.go
new file mode 100644
index 00000000..abd03479
--- /dev/null
+++ b/setup/setup_test.go
@@ -0,0 +1,111 @@
+/*
+ * setup_test.go
+ *
+ * This source file is part of the FoundationDB open source project
+ *
+ * Copyright 2023 Apple Inc. and the FoundationDB project authors
+ *
+ * Licensed 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 setup
+
+import (
+	"io/fs"
+	"os"
+	"path"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("setup", func() {
+	var options Options
+
+	When("no log output file is defined", func() {
+		It("should return stdout as writer", func() {
+			writer, err := setupLogger(options)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(writer).To(BeIdenticalTo(os.Stdout))
+		})
+	})
+
+	When("a log output file is defined", func() {
+		var tmpDir, logFile string
+
+		BeforeEach(func() {
+			tmpDir = GinkgoT().TempDir()
+			logFile = path.Join(tmpDir, "operator.logs")
+
+			options = Options{
+				LogFile: logFile,
+			}
+		})
+
+		It("should create the log file with the right permissions", func() {
+			writer, err := setupLogger(options)
+			Expect(err).NotTo(HaveOccurred())
+
+			_, err = writer.Write([]byte("Hello World!"))
+			Expect(err).NotTo(HaveOccurred())
+
+			resultFile, err := os.Stat(logFile)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(logFile).To(Equal(path.Join(tmpDir, resultFile.Name())))
+			// Default file mode is 0644
+			Expect(resultFile.Mode()).To(Equal(fs.FileMode(0644)))
+			Expect(resultFile.Size()).To(BeNumerically(">", 0))
+		})
+
+		When("the log file already exists with the wrong permissions", func() {
+			BeforeEach(func() {
+				Expect(os.WriteFile(logFile, nil, 0600)).NotTo(HaveOccurred())
+			})
+
+			It("should correct the permission", func() {
+				writer, err := setupLogger(options)
+				Expect(err).NotTo(HaveOccurred())
+
+				_, err = writer.Write([]byte("Hello World!"))
+				Expect(err).NotTo(HaveOccurred())
+
+				resultFile, err := os.Stat(logFile)
+				Expect(err).NotTo(HaveOccurred())
+				Expect(logFile).To(Equal(path.Join(tmpDir, resultFile.Name())))
+				// Default file mode is 0644
+				Expect(resultFile.Mode()).To(Equal(fs.FileMode(0644)))
+				Expect(resultFile.Size()).To(BeNumerically(">", 0))
+			})
+		})
+
+		When("file permissions are specified", func() {
+			BeforeEach(func() {
+				options.LogFilePermission = "0600"
+			})
+
+			It("should correct the permission", func() {
+				writer, err := setupLogger(options)
+				Expect(err).NotTo(HaveOccurred())
+
+				_, err = writer.Write([]byte("Hello World!"))
+				Expect(err).NotTo(HaveOccurred())
+
+				resultFile, err := os.Stat(logFile)
+				Expect(err).NotTo(HaveOccurred())
+				Expect(logFile).To(Equal(path.Join(tmpDir, resultFile.Name())))
+				Expect(resultFile.Mode()).To(Equal(fs.FileMode(0600)))
+				Expect(resultFile.Size()).To(BeNumerically(">", 0))
+			})
+		})
+	})
+})
diff --git a/setup/suite_test.go b/setup/suite_test.go
new file mode 100644
index 00000000..a1aeb179
--- /dev/null
+++ b/setup/suite_test.go
@@ -0,0 +1,33 @@
+/*
+ * suite_test.go
+ *
+ * This source file is part of the FoundationDB open source project
+ *
+ * Copyright 2023 Apple Inc. and the FoundationDB project authors
+ *
+ * Licensed 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 setup
+
+import (
+	"testing"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+func TestCmd(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "setup")
+}