From 4688eb2801c594b1211043189f5b9ee0620eb615 Mon Sep 17 00:00:00 2001 From: Matthew Cary Date: Fri, 21 Jul 2023 14:28:49 -0700 Subject: [PATCH] Deprecate kubelet-registration-probe and remove an unneccesary write to the root filesystem Change-Id: Id040a6a4a9248b1fbc2472132c098f2787b2f0e8 --- cmd/csi-node-driver-registrar/main.go | 37 ++------------- .../node_register.go | 6 --- pkg/util/util_test.go | 39 ---------------- pkg/util/util_unix.go | 45 ------------------- pkg/util/util_windows.go | 45 ------------------- 5 files changed, 3 insertions(+), 169 deletions(-) diff --git a/cmd/csi-node-driver-registrar/main.go b/cmd/csi-node-driver-registrar/main.go index 67c41064f..bd91385c5 100644 --- a/cmd/csi-node-driver-registrar/main.go +++ b/cmd/csi-node-driver-registrar/main.go @@ -22,12 +22,10 @@ import ( "fmt" _ "net/http/pprof" "os" - "path/filepath" "strconv" "time" "github.com/kubernetes-csi/csi-lib-utils/metrics" - "github.com/kubernetes-csi/node-driver-registrar/pkg/util" "k8s.io/klog/v2" "github.com/kubernetes-csi/csi-lib-utils/connection" @@ -45,20 +43,11 @@ const ( ) const ( - // ModeRegistration runs node-driver-registrar as a long running process - ModeRegistration = "registration" - // ModeKubeletRegistrationProbe makes node-driver-registrar act as an exec probe // that checks if the kubelet plugin registration succeeded. ModeKubeletRegistrationProbe = "kubelet-registration-probe" ) -var ( - // The registration probe path, set when the program runs and used as the path of the file - // to create when the kubelet plugin registration succeeds. - registrationProbePath = "" -) - // Command line flags var ( connectionTimeout = flag.Duration("connection-timeout", 0, "The --connection-timeout flag is deprecated") @@ -69,7 +58,7 @@ var ( healthzPort = flag.Int("health-port", 0, "(deprecated) TCP port for healthz requests. Set to 0 to disable the healthz server. Only one of `--health-port` and `--http-endpoint` can be set.") httpEndpoint = flag.String("http-endpoint", "", "The TCP network address where the HTTP server for diagnostics, including pprof and the health check indicating whether the registration socket exists, will listen (example: `:8080`). The default is empty string, which means the server is disabled. Only one of `--health-port` and `--http-endpoint` can be set.") showVersion = flag.Bool("version", false, "Show version.") - mode = flag.String("mode", ModeRegistration, `The running mode of node-driver-registrar. "registration" runs node-driver-registrar as a long running process. "kubelet-registration-probe" runs as a health check and returns a status code of 0 if the driver was registered successfully, in the probe definition make sure that the value of --kubelet-registration-path is the same as in the container.`) + mode = flag.String("mode", "", "DEPRECATED. This flag is no longer used. If this is set to kubelet-registration-probe, the driver will immediately exit successfully without registering with CSI.") enableProfile = flag.Bool("enable-pprof", false, "enable pprof profiling") // Set during compilation time @@ -101,14 +90,6 @@ func newRegistrationServer(driverName string, endpoint string, versions []string func (e registrationServer) GetInfo(ctx context.Context, req *registerapi.InfoRequest) (*registerapi.PluginInfo, error) { klog.Infof("Received GetInfo call: %+v", req) - // on successful registration, create the registration probe file - err := util.TouchFile(registrationProbePath) - if err != nil { - klog.ErrorS(err, "Failed to create registration probe file", "registrationProbePath", registrationProbePath) - } else { - klog.InfoS("Kubelet registration probe created", "path", registrationProbePath) - } - return ®isterapi.PluginInfo{ Type: registerapi.CSIPlugin, Name: e.driverName, @@ -145,22 +126,10 @@ func main() { klog.Error("kubelet-registration-path is a required parameter") os.Exit(1) } - // set after we made sure that *kubeletRegistrationPath exists - kubeletRegistrationPathDir := filepath.Dir(*kubeletRegistrationPath) - registrationProbePath = filepath.Join(kubeletRegistrationPathDir, "registration") - // with the mode kubelet-registration-probe if modeIsKubeletRegistrationProbe() { - lockfileExists, err := util.DoesFileExist(registrationProbePath) - if err != nil { - klog.Fatalf("Failed to check if registration path exists, registrationProbePath=%s err=%v", registrationProbePath, err) - os.Exit(1) - } - if !lockfileExists { - klog.Fatalf("Kubelet plugin registration hasn't succeeded yet, file=%s doesn't exist.", registrationProbePath) - os.Exit(1) - } - klog.Infof("Kubelet plugin registration succeeded.") + // This is no longer needed, see https://github.com/kubernetes-csi/node-driver-registrar/issues/309. + // In case this mode is still in use, it will succeed as a no-op. os.Exit(0) } diff --git a/cmd/csi-node-driver-registrar/node_register.go b/cmd/csi-node-driver-registrar/node_register.go index 54ba1f187..303f5c25e 100644 --- a/cmd/csi-node-driver-registrar/node_register.go +++ b/cmd/csi-node-driver-registrar/node_register.go @@ -62,10 +62,6 @@ func nodeRegister(csiDriverName, httpEndpoint string) { klog.Infof("Registration Server started at: %s\n", socketPath) grpcServer := grpc.NewServer() - // Before registering node-driver-registrar with the kubelet ensure that the lockfile doesn't exist - // a lockfile may exist because the container was forcefully shutdown - util.CleanupFile(registrationProbePath) - // Registers kubelet plugin watcher api. registerapi.RegisterRegistrationServer(grpcServer, registrar) @@ -77,8 +73,6 @@ func nodeRegister(csiDriverName, httpEndpoint string) { os.Exit(1) } - // clean the file on graceful shutdown - util.CleanupFile(registrationProbePath) // If gRPC server is gracefully shutdown, cleanup and exit os.Exit(0) } diff --git a/pkg/util/util_test.go b/pkg/util/util_test.go index 87d54554d..298b168ae 100644 --- a/pkg/util/util_test.go +++ b/pkg/util/util_test.go @@ -28,7 +28,6 @@ import ( ) var socketFileName = "reg.sock" -var kubeletRegistrationPath = "/var/lib/kubelet/plugins/csi-dummy/registration" // TestSocketFileDoesNotExist - Test1: file does not exist. So clean up should be successful. func TestSocketFileDoesNotExist(t *testing.T) { @@ -174,41 +173,3 @@ func TestSocketRegularFile(t *testing.T) { } } } - -// TestTouchFile creates a file if it doesn't exist -func TestTouchFile(t *testing.T) { - // Create a temp directory - testDir, err := utiltesting.MkTmpdir("csi-test") - if err != nil { - t.Fatalf("could not create temp dir: %v", err) - } - defer os.RemoveAll(testDir) - - filePath := filepath.Join(testDir, kubeletRegistrationPath) - fileExists, err := DoesFileExist(filePath) - if err != nil { - t.Fatalf("Failed to execute file exist: %+v", err) - } - if fileExists { - t.Fatalf("File %s must not exist", filePath) - } - - // returns an error only if it failed to clean the file, not if the file didn't exist - err = CleanupFile(filePath) - if err != nil { - t.Fatalf("Failed to execute file cleanup: %+v", err) - } - - err = TouchFile(filePath) - if err != nil { - t.Fatalf("Failed to execute file touch: %+v", err) - } - - fileExists, err = DoesFileExist(filePath) - if err != nil { - t.Fatalf("Failed to execute file exist: %+v", err) - } - if !fileExists { - t.Fatalf("File %s must exist", filePath) - } -} diff --git a/pkg/util/util_unix.go b/pkg/util/util_unix.go index 3a2da755f..511a210b0 100644 --- a/pkg/util/util_unix.go +++ b/pkg/util/util_unix.go @@ -22,7 +22,6 @@ package util import ( "fmt" "os" - "path/filepath" "golang.org/x/sys/unix" ) @@ -57,47 +56,3 @@ func DoesSocketExist(socketPath string) (bool, error) { } return false, nil } - -func CleanupFile(filePath string) error { - fileExists, err := DoesFileExist(filePath) - if err != nil { - return err - } - if fileExists { - if err := os.Remove(filePath); err != nil { - return fmt.Errorf("failed to remove stale file=%s with error: %+v", filePath, err) - } - } - return nil -} - -func DoesFileExist(filePath string) (bool, error) { - info, err := os.Stat(filePath) - if err == nil { - return info.Mode().IsRegular(), nil - } - if err != nil && !os.IsNotExist(err) { - return false, fmt.Errorf("Failed to stat the file=%s with error: %+v", filePath, err) - } - return false, nil -} - -func TouchFile(filePath string) error { - exists, err := DoesFileExist(filePath) - if err != nil { - return err - } - if !exists { - err := os.MkdirAll(filepath.Dir(filePath), 0755) - if err != nil { - return err - } - - file, err := os.Create(filePath) - if err != nil { - return err - } - file.Close() - } - return nil -} diff --git a/pkg/util/util_windows.go b/pkg/util/util_windows.go index e8f2a236e..9821c9d1d 100644 --- a/pkg/util/util_windows.go +++ b/pkg/util/util_windows.go @@ -23,7 +23,6 @@ import ( "errors" "fmt" "os" - "path/filepath" ) func Umask(mask int) (int, error) { @@ -55,47 +54,3 @@ func DoesSocketExist(socketPath string) (bool, error) { } return true, nil } - -func CleanupFile(filePath string) error { - fileExists, err := DoesFileExist(filePath) - if err != nil { - return err - } - if fileExists { - if err := os.Remove(filePath); err != nil { - return fmt.Errorf("failed to remove stale file=%s with error: %+v", filePath, err) - } - } - return nil -} - -func DoesFileExist(filePath string) (bool, error) { - info, err := os.Lstat(filePath) - if err == nil { - return info.Mode().IsRegular(), nil - } - if err != nil && !os.IsNotExist(err) { - return false, fmt.Errorf("Failed to stat the file=%s with error: %+v", filePath, err) - } - return false, nil -} - -func TouchFile(filePath string) error { - exists, err := DoesFileExist(filePath) - if err != nil { - return err - } - if !exists { - err := os.MkdirAll(filepath.Dir(filePath), 0755) - if err != nil { - return err - } - - file, err := os.Create(filePath) - if err != nil { - return err - } - file.Close() - } - return nil -}