Merge pull request #316 from mattcary/root

Deprecate kubelet-registration-probe

README.md

@@ -60,7 +60,7 @@ There are two UNIX domain sockets used by the node-driver-registrar:
 
 * `--timeout <duration>`: Timeout of all calls to CSI driver. It should be set to a value that accommodates the `GetDriverName` calls. 1 second is used by default.
 
-* `--mode <mode>` (default: `--mode=registration`): The running mode of node-driver-registrar. `registration` runs node-driver-registrar as a long running process to register the driver with kubelet. `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 <mode>` (default: `--mode=registration`): DEPRECATED. If this is set to kubelet-registration-probe, the driver will exit successfully without registering with CSI. If set to any other value node-driver-registrar will do the kubelet plugin registration. This flag will be removed in a future major release because the mode kubelet-registration-probe is no longer needed.
 
 * `--enable-pprof`: Enable pprof profiling on the TCP network address specified by `--http-endpoint`.
 

cmd/csi-node-driver-registrar/main.go

@@ -22,11 +22,9 @@ import (
 	"fmt"
 	_ "net/http/pprof"
 	"os"
-	"path/filepath"
 	"strconv"
 	"time"
 
-	"github.com/kubernetes-csi/node-driver-registrar/pkg/util"
 	"k8s.io/klog/v2"
 
 	"github.com/kubernetes-csi/csi-lib-utils/connection"
@@ -44,20 +42,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")
@@ -68,7 +57,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. If this is set to kubelet-registration-probe, the driver will exit successfully without registering with CSI. If set to any other value node-driver-registrar will do the kubelet plugin registration. This flag will be removed in a future major release because the mode kubelet-registration-probe is no longer needed.")
 	enableProfile           = flag.Bool("enable-pprof", false, "enable pprof profiling")
 
 	// Set during compilation time
@@ -100,14 +89,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 &registerapi.PluginInfo{
 		Type:              registerapi.CSIPlugin,
 		Name:              e.driverName,
@@ -144,22 +125,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)
 	}
 

cmd/csi-node-driver-registrar/node_register.go

@@ -64,10 +64,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)
 
@@ -79,8 +75,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)
 }

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)
-	}
-}

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
-}

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
-}