Skip to content

Commit

Permalink
Deprecate kubelet-registration-probe and remove an unneccesary write …
Browse files Browse the repository at this point in the history
…to the root filesystem

Change-Id: I42f436e7295adf708df15e57d0c24a5175c67495
  • Loading branch information
mattcary committed Aug 28, 2023
1 parent c9e19f2 commit 25eb1c9
Show file tree
Hide file tree
Showing 6 changed files with 4 additions and 170 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.

Expand Down
37 changes: 3 additions & 34 deletions cmd/csi-node-driver-registrar/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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")
Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
}

Expand Down
6 changes: 0 additions & 6 deletions cmd/csi-node-driver-registrar/node_register.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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)
}
Expand Down
39 changes: 0 additions & 39 deletions pkg/util/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
}
}
45 changes: 0 additions & 45 deletions pkg/util/util_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ package util
import (
"fmt"
"os"
"path/filepath"

"golang.org/x/sys/unix"
)
Expand Down Expand Up @@ -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
}
45 changes: 0 additions & 45 deletions pkg/util/util_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"errors"
"fmt"
"os"
"path/filepath"
)

func Umask(mask int) (int, error) {
Expand Down Expand Up @@ -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
}

0 comments on commit 25eb1c9

Please sign in to comment.