kubeadm: detect runtime socket as URL format

- Update defaults for v1beta2 and 3 to have URL scheme
- Raname DefaultUrlScheme to DefaultContainerRuntimeURLScheme
- Prepend a missing URL scheme to user sockets and warn them
that this might not be supported in the future
- Update socket validation to exclude IsAbs() testing
(This is broken on Windows). Assume the path is not empty and has
URL scheme at this point (validation happens after defaulting).
- Use net.Dial to open Unix sockets
- Update all related unit tests

Signed-off-by: pacoxu <paco.xu@daocloud.io>
Signed-off-by: Lubomir I. Ivanov <lubomirivanov@vmware.com>
This commit is contained in:
pacoxu 2021-06-11 16:05:41 +08:00 committed by Lubomir I. Ivanov
parent 7c013c3f64
commit 7594f0ef90
18 changed files with 99 additions and 84 deletions

View File

@ -22,6 +22,6 @@ package v1beta2
const (
// DefaultCACertPath defines default location of CA certificate on Linux
DefaultCACertPath = "/etc/kubernetes/pki/ca.crt"
// DefaultUrlScheme defines default socket url prefix
DefaultUrlScheme = "unix"
// DefaultContainerRuntimeURLScheme defines default socket url prefix
DefaultContainerRuntimeURLScheme = "unix"
)

View File

@ -22,6 +22,6 @@ package v1beta2
const (
// DefaultCACertPath defines default location of CA certificate on Windows
DefaultCACertPath = "C:/etc/kubernetes/pki/ca.crt"
// DefaultUrlScheme defines default socket url prefix
DefaultUrlScheme = "npipe"
// DefaultContainerRuntimeURLScheme defines default socket url prefix
DefaultContainerRuntimeURLScheme = "npipe"
)

View File

@ -169,7 +169,7 @@ limitations under the License.
// - system:bootstrappers:kubeadm:default-node-token
// nodeRegistration:
// name: "ec2-10-100-0-1"
// criSocket: "/var/run/dockershim.sock"
// criSocket: "unix:///var/run/dockershim.sock"
// taints:
// - key: "kubeadmNode"
// value: "master"

View File

@ -22,6 +22,6 @@ package v1beta3
const (
// DefaultCACertPath defines default location of CA certificate on Linux
DefaultCACertPath = "/etc/kubernetes/pki/ca.crt"
// DefaultUrlScheme defines default socket url prefix
DefaultUrlScheme = "unix"
// DefaultContainerRuntimeURLScheme defines default socket url prefix
DefaultContainerRuntimeURLScheme = "unix"
)

View File

@ -22,6 +22,6 @@ package v1beta3
const (
// DefaultCACertPath defines default location of CA certificate on Windows
DefaultCACertPath = "C:/etc/kubernetes/pki/ca.crt"
// DefaultUrlScheme defines default socket url prefix
DefaultUrlScheme = "npipe"
// DefaultContainerRuntimeURLScheme defines default socket url prefix
DefaultContainerRuntimeURLScheme = "npipe"
)

View File

@ -173,7 +173,7 @@ limitations under the License.
// - system:bootstrappers:kubeadm:default-node-token
// nodeRegistration:
// name: "ec2-10-100-0-1"
// criSocket: "/var/run/dockershim.sock"
// criSocket: "unix:///var/run/dockershim.sock"
// taints:
// - key: "kubeadmNode"
// value: "master"

View File

@ -624,17 +624,18 @@ func ValidateIgnorePreflightErrors(ignorePreflightErrorsFromCLI, ignorePreflight
func ValidateSocketPath(socket string, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
if len(socket) == 0 { // static and dynamic defaulting should have added a value to the field already
return append(allErrs, field.Invalid(fldPath, socket, "empty CRI socket"))
}
u, err := url.Parse(socket)
if err != nil {
return append(allErrs, field.Invalid(fldPath, socket, fmt.Sprintf("URL parsing error: %v", err)))
}
if u.Scheme == "" {
if !filepath.IsAbs(u.Path) {
return append(allErrs, field.Invalid(fldPath, socket, fmt.Sprintf("path is not absolute: %s", socket)))
}
} else if u.Scheme != kubeadmapiv1.DefaultUrlScheme {
return append(allErrs, field.Invalid(fldPath, socket, fmt.Sprintf("URL scheme %s is not supported", u.Scheme)))
// static and dynamic defaulting should have ensured that an URL scheme is used
if u.Scheme != kubeadmapiv1.DefaultContainerRuntimeURLScheme {
return append(allErrs, field.Invalid(fldPath, socket, fmt.Sprintf("only URL scheme %q is supported, got %q", kubeadmapiv1.DefaultContainerRuntimeURLScheme, u.Scheme)))
}
return allErrs

View File

@ -111,7 +111,7 @@ func TestValidateNodeRegistrationOptions(t *testing.T) {
// test cases for criSocket are covered in TestValidateSocketPath
}
for _, rt := range tests {
nro := kubeadmapi.NodeRegistrationOptions{Name: rt.nodeName, CRISocket: "/some/path"}
nro := kubeadmapi.NodeRegistrationOptions{Name: rt.nodeName, CRISocket: "unix:///some/path"}
actual := ValidateNodeRegistrationOptions(&nro, field.NewPath("nodeRegistration"))
actualErrors := len(actual) > 0
if actualErrors != rt.expectedErrors {
@ -448,7 +448,7 @@ func TestValidateInitConfiguration(t *testing.T) {
},
CertificatesDir: "/some/cert/dir",
},
NodeRegistration: kubeadmapi.NodeRegistrationOptions{Name: nodename, CRISocket: "/some/path"},
NodeRegistration: kubeadmapi.NodeRegistrationOptions{Name: nodename, CRISocket: "unix:///some/path"},
}, false},
{"invalid missing token with IPv6 service subnet",
&kubeadmapi.InitConfiguration{
@ -463,7 +463,7 @@ func TestValidateInitConfiguration(t *testing.T) {
},
CertificatesDir: "/some/cert/dir",
},
NodeRegistration: kubeadmapi.NodeRegistrationOptions{Name: nodename, CRISocket: "/some/path"},
NodeRegistration: kubeadmapi.NodeRegistrationOptions{Name: nodename, CRISocket: "unix:///some/path"},
}, false},
{"invalid missing node name",
&kubeadmapi.InitConfiguration{
@ -493,7 +493,7 @@ func TestValidateInitConfiguration(t *testing.T) {
},
CertificatesDir: "/some/other/cert/dir",
},
NodeRegistration: kubeadmapi.NodeRegistrationOptions{Name: nodename, CRISocket: "/some/path"},
NodeRegistration: kubeadmapi.NodeRegistrationOptions{Name: nodename, CRISocket: "unix:///some/path"},
}, false},
{"valid InitConfiguration with IPv4 service subnet",
&kubeadmapi.InitConfiguration{
@ -514,7 +514,7 @@ func TestValidateInitConfiguration(t *testing.T) {
},
CertificatesDir: "/some/other/cert/dir",
},
NodeRegistration: kubeadmapi.NodeRegistrationOptions{Name: nodename, CRISocket: "/some/path"},
NodeRegistration: kubeadmapi.NodeRegistrationOptions{Name: nodename, CRISocket: "unix:///some/path"},
}, true},
{"valid InitConfiguration using IPv6 service subnet",
&kubeadmapi.InitConfiguration{
@ -534,7 +534,7 @@ func TestValidateInitConfiguration(t *testing.T) {
},
CertificatesDir: "/some/other/cert/dir",
},
NodeRegistration: kubeadmapi.NodeRegistrationOptions{Name: nodename, CRISocket: "/some/path"},
NodeRegistration: kubeadmapi.NodeRegistrationOptions{Name: nodename, CRISocket: "unix:///some/path"},
}, true},
}
for _, rt := range tests {
@ -579,7 +579,7 @@ func TestValidateJoinConfiguration(t *testing.T) {
},
NodeRegistration: kubeadmapi.NodeRegistrationOptions{
Name: "aaa",
CRISocket: "/var/run/dockershim.sock",
CRISocket: "unix:///var/run/dockershim.sock",
},
}, true},
{&kubeadmapi.JoinConfiguration{ // Pass with JoinControlPlane
@ -594,7 +594,7 @@ func TestValidateJoinConfiguration(t *testing.T) {
},
NodeRegistration: kubeadmapi.NodeRegistrationOptions{
Name: "aaa",
CRISocket: "/var/run/dockershim.sock",
CRISocket: "unix:///var/run/dockershim.sock",
},
ControlPlane: &kubeadmapi.JoinControlPlane{
LocalAPIEndpoint: kubeadmapi.APIEndpoint{
@ -615,7 +615,7 @@ func TestValidateJoinConfiguration(t *testing.T) {
},
NodeRegistration: kubeadmapi.NodeRegistrationOptions{
Name: "aaa",
CRISocket: "/var/run/dockershim.sock",
CRISocket: "unix:///var/run/dockershim.sock",
},
ControlPlane: &kubeadmapi.JoinControlPlane{
LocalAPIEndpoint: kubeadmapi.APIEndpoint{
@ -963,12 +963,11 @@ func TestValidateSocketPath(t *testing.T) {
criSocket string
expectedErrors bool
}{
{name: "valid path", criSocket: "/some/path", expectedErrors: false},
{name: "valid socket url", criSocket: kubeadmapiv1.DefaultUrlScheme + "://" + "/some/path", expectedErrors: false},
{name: "unsupported url scheme", criSocket: "bla:///some/path", expectedErrors: true},
{name: "unparseable url", criSocket: ":::", expectedErrors: true},
{name: "invalid CRISocket (path is not absolute)", criSocket: "some/path", expectedErrors: true},
{name: "empty CRISocket (path is not absolute)", criSocket: "", expectedErrors: true},
{name: "valid socket URL", criSocket: kubeadmapiv1.DefaultContainerRuntimeURLScheme + "://" + "/some/path", expectedErrors: false},
{name: "unsupported URL scheme", criSocket: "bla:///some/path", expectedErrors: true},
{name: "missing URL scheme", criSocket: "/some/path", expectedErrors: true},
{name: "unparseable URL", criSocket: ":::", expectedErrors: true},
{name: "empty CRISocket", criSocket: "", expectedErrors: true},
}
for _, tc := range tests {
actual := ValidateSocketPath(tc.criSocket, field.NewPath("criSocket"))

View File

@ -245,7 +245,7 @@ const (
AnnotationKubeadmCRISocket = "kubeadm.alpha.kubernetes.io/cri-socket"
// UnknownCRISocket defines the undetected or unknown CRI socket
UnknownCRISocket = "/var/run/unknown.sock"
UnknownCRISocket = "unix:///var/run/unknown.sock"
// KubeadmConfigConfigMap specifies in what ConfigMap in the kube-system namespace the `kubeadm init` configuration should be stored
KubeadmConfigConfigMap = "kubeadm-config"

View File

@ -21,5 +21,5 @@ package constants
const (
// DefaultDockerCRISocket defines the default Docker CRI socket
DefaultDockerCRISocket = "/var/run/dockershim.sock"
DefaultDockerCRISocket = "unix:///var/run/dockershim.sock"
)

View File

@ -38,7 +38,7 @@ func TestBuildKubeletArgMap(t *testing.T) {
name: "the simplest case",
opts: kubeletFlagsOpts{
nodeRegOpts: &kubeadmapi.NodeRegistrationOptions{
CRISocket: "/var/run/dockershim.sock",
CRISocket: "unix:///var/run/dockershim.sock",
Taints: []v1.Taint{ // This should be ignored as registerTaintsUsingFlags is false
{
Key: "foo",
@ -56,7 +56,7 @@ func TestBuildKubeletArgMap(t *testing.T) {
name: "hostname override from NodeRegistrationOptions.Name",
opts: kubeletFlagsOpts{
nodeRegOpts: &kubeadmapi.NodeRegistrationOptions{
CRISocket: "/var/run/dockershim.sock",
CRISocket: "unix:///var/run/dockershim.sock",
Name: "override-name",
},
},
@ -69,7 +69,7 @@ func TestBuildKubeletArgMap(t *testing.T) {
name: "hostname override from NodeRegistrationOptions.KubeletExtraArgs",
opts: kubeletFlagsOpts{
nodeRegOpts: &kubeadmapi.NodeRegistrationOptions{
CRISocket: "/var/run/dockershim.sock",
CRISocket: "unix:///var/run/dockershim.sock",
KubeletExtraArgs: map[string]string{"hostname-override": "override-name"},
},
},
@ -82,19 +82,19 @@ func TestBuildKubeletArgMap(t *testing.T) {
name: "external CRI runtime",
opts: kubeletFlagsOpts{
nodeRegOpts: &kubeadmapi.NodeRegistrationOptions{
CRISocket: "/var/run/containerd.sock",
CRISocket: "unix:///var/run/containerd/containerd.sock",
},
},
expected: map[string]string{
"container-runtime": "remote",
"container-runtime-endpoint": "/var/run/containerd.sock",
"container-runtime-endpoint": "unix:///var/run/containerd/containerd.sock",
},
},
{
name: "register with taints",
opts: kubeletFlagsOpts{
nodeRegOpts: &kubeadmapi.NodeRegistrationOptions{
CRISocket: "/var/run/containerd.sock",
CRISocket: "unix:///var/run/containerd/containerd.sock",
Taints: []v1.Taint{
{
Key: "foo",
@ -112,7 +112,7 @@ func TestBuildKubeletArgMap(t *testing.T) {
},
expected: map[string]string{
"container-runtime": "remote",
"container-runtime-endpoint": "/var/run/containerd.sock",
"container-runtime-endpoint": "unix:///var/run/containerd/containerd.sock",
"register-with-taints": "foo=bar:baz,key=val:eff",
},
},
@ -120,7 +120,7 @@ func TestBuildKubeletArgMap(t *testing.T) {
name: "pause image is set",
opts: kubeletFlagsOpts{
nodeRegOpts: &kubeadmapi.NodeRegistrationOptions{
CRISocket: "/var/run/dockershim.sock",
CRISocket: "unix:///var/run/dockershim.sock",
},
pauseImage: "k8s.gcr.io/pause:3.6",
},
@ -133,7 +133,7 @@ func TestBuildKubeletArgMap(t *testing.T) {
name: "dockershim socket and kubelet version with built-in dockershim",
opts: kubeletFlagsOpts{
nodeRegOpts: &kubeadmapi.NodeRegistrationOptions{
CRISocket: "/var/run/dockershim.sock",
CRISocket: "unix:///var/run/dockershim.sock",
},
kubeletVersion: version.MustParseSemantic("v1.23.6"),
},
@ -145,13 +145,13 @@ func TestBuildKubeletArgMap(t *testing.T) {
name: "dockershim socket but kubelet version is without built-in dockershim",
opts: kubeletFlagsOpts{
nodeRegOpts: &kubeadmapi.NodeRegistrationOptions{
CRISocket: "/var/run/dockershim.sock",
CRISocket: "unix:///var/run/dockershim.sock",
},
kubeletVersion: version.MustParseSemantic("v1.24.0-alpha.1"),
},
expected: map[string]string{
"container-runtime": "remote",
"container-runtime-endpoint": "/var/run/dockershim.sock",
"container-runtime-endpoint": "unix:///var/run/dockershim.sock",
},
},
}

View File

@ -41,20 +41,20 @@ func TestAnnotateCRISocket(t *testing.T) {
{
name: "CRI-socket annotation missing",
currentCRISocketAnnotation: "",
newCRISocketAnnotation: "/run/containerd/containerd.sock",
expectedPatch: `{"metadata":{"annotations":{"kubeadm.alpha.kubernetes.io/cri-socket":"/run/containerd/containerd.sock"}}}`,
newCRISocketAnnotation: "unix:///run/containerd/containerd.sock",
expectedPatch: `{"metadata":{"annotations":{"kubeadm.alpha.kubernetes.io/cri-socket":"unix:///run/containerd/containerd.sock"}}}`,
},
{
name: "CRI-socket annotation already exists",
currentCRISocketAnnotation: "/run/containerd/containerd.sock",
newCRISocketAnnotation: "/run/containerd/containerd.sock",
currentCRISocketAnnotation: "unix:///run/containerd/containerd.sock",
newCRISocketAnnotation: "unix:///run/containerd/containerd.sock",
expectedPatch: `{}`,
},
{
name: "CRI-socket annotation needs to be updated",
currentCRISocketAnnotation: "/var/run/dockershim.sock",
newCRISocketAnnotation: "/run/containerd/containerd.sock",
expectedPatch: `{"metadata":{"annotations":{"kubeadm.alpha.kubernetes.io/cri-socket":"/run/containerd/containerd.sock"}}}`,
currentCRISocketAnnotation: "unix:///var/run/dockershim.sock",
newCRISocketAnnotation: "unix:///run/containerd/containerd.sock",
expectedPatch: `{"metadata":{"annotations":{"kubeadm.alpha.kubernetes.io/cri-socket":"unix:///run/containerd/containerd.sock"}}}`,
},
}

View File

@ -22,6 +22,7 @@ import (
"io/ioutil"
"net"
"strconv"
"strings"
"github.com/pkg/errors"
@ -115,6 +116,13 @@ func SetNodeRegistrationDynamicDefaults(cfg *kubeadmapi.NodeRegistrationOptions,
return err
}
klog.V(1).Infof("detected and using CRI socket: %s", cfg.CRISocket)
} else {
if !strings.HasPrefix(cfg.CRISocket, kubeadmapiv1.DefaultContainerRuntimeURLScheme) {
klog.Warningf("Usage of CRI endpoints without URL scheme is deprecated and can cause kubelet errors "+
"in the future. Automatically prepending scheme %q to the \"criSocket\" with value %q. "+
"Please update your configuration!", kubeadmapiv1.DefaultContainerRuntimeURLScheme, cfg.CRISocket)
cfg.CRISocket = kubeadmapiv1.DefaultContainerRuntimeURLScheme + "://" + cfg.CRISocket
}
}
return nil

View File

@ -119,7 +119,7 @@ func TestMarshalUnmarshalToYamlForCodecs(t *testing.T) {
},
NodeRegistration: kubeadmapiv1.NodeRegistrationOptions{
Name: "testNode",
CRISocket: "/var/run/cri.sock",
CRISocket: "unix:///var/run/cri.sock",
},
BootstrapTokens: []bootstraptokenv1.BootstrapToken{
{

View File

@ -14,11 +14,9 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
package util
package runtime
import (
"path/filepath"
goruntime "runtime"
"strings"
"github.com/pkg/errors"
@ -57,12 +55,6 @@ func NewContainerRuntime(execer utilsexec.Interface, criSocket string) (Containe
if criSocket != constants.DefaultDockerCRISocket {
toolName = "crictl"
// !!! temporary work around crictl warning:
// Using "/var/run/crio/crio.sock" as endpoint is deprecated,
// please consider using full url format "unix:///var/run/crio/crio.sock"
if filepath.IsAbs(criSocket) && goruntime.GOOS != "windows" {
criSocket = "unix://" + criSocket
}
runtime = &CRIRuntime{execer, criSocket}
} else {
toolName = "docker"
@ -198,7 +190,7 @@ func detectCRISocketImpl(isSocket func(string) bool) (string, error) {
foundCRISockets := []string{}
knownCRISockets := []string{
// Docker and containerd sockets are special cased below, hence not to be included here
"/var/run/crio/crio.sock",
"unix:///var/run/crio/crio.sock",
}
if isSocket(dockerSocket) {

View File

@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
package util
package runtime
import (
"io/ioutil"
@ -48,7 +48,6 @@ func TestNewContainerRuntime(t *testing.T) {
}{
{"valid: default cri socket", execLookPathOK, constants.DefaultDockerCRISocket, true, false},
{"valid: cri-o socket url", execLookPathOK, "unix:///var/run/crio/crio.sock", false, false},
{"valid: cri-o socket path", execLookPathOK, "/var/run/crio/crio.sock", false, false},
{"invalid: no crictl", execLookPathErr, "unix:///var/run/crio/crio.sock", false, true},
}
@ -351,7 +350,7 @@ func TestIsExistingSocket(t *testing.T) {
}
defer con.Close()
if !isExistingSocket(theSocket) {
if !isExistingSocket("unix://" + theSocket) {
t.Fatalf("isExistingSocket(%q) gave unexpected result. Should have been true, instead of false", theSocket)
}
},
@ -403,29 +402,29 @@ func TestDetectCRISocketImpl(t *testing.T) {
},
{
name: "One valid CRI socket leads to success",
existingSockets: []string{"/var/run/crio/crio.sock"},
existingSockets: []string{"unix:///var/run/crio/crio.sock"},
expectedError: false,
expectedSocket: "/var/run/crio/crio.sock",
expectedSocket: "unix:///var/run/crio/crio.sock",
},
{
name: "Correct Docker CRI socket is returned",
existingSockets: []string{"/var/run/docker.sock"},
existingSockets: []string{"unix:///var/run/docker.sock"},
expectedError: false,
expectedSocket: constants.DefaultDockerCRISocket,
},
{
name: "CRI and Docker sockets lead to an error",
existingSockets: []string{
"/var/run/docker.sock",
"/var/run/crio/crio.sock",
"unix:///var/run/docker.sock",
"unix:///var/run/crio/crio.sock",
},
expectedError: true,
},
{
name: "Docker and containerd lead to Docker being used",
existingSockets: []string{
"/var/run/docker.sock",
"/run/containerd/containerd.sock",
"unix:///var/run/docker.sock",
"unix:///run/containerd/containerd.sock",
},
expectedError: false,
expectedSocket: constants.DefaultDockerCRISocket,
@ -433,8 +432,8 @@ func TestDetectCRISocketImpl(t *testing.T) {
{
name: "A couple of CRI sockets lead to an error",
existingSockets: []string{
"/var/run/crio/crio.sock",
"/run/containerd/containerd.sock",
"unix:///var/run/crio/crio.sock",
"unix:///run/containerd/containerd.sock",
},
expectedError: true,
},

View File

@ -17,23 +17,30 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
package util
package runtime
import (
"os"
"net"
"net/url"
)
const (
dockerSocket = "/var/run/docker.sock" // The Docker socket is not CRI compatible
containerdSocket = "/run/containerd/containerd.sock"
dockerSocket = "unix:///var/run/docker.sock" // The Docker socket is not CRI compatible
containerdSocket = "unix:///run/containerd/containerd.sock"
)
// isExistingSocket checks if path exists and is domain socket
func isExistingSocket(path string) bool {
fileInfo, err := os.Stat(path)
u, err := url.Parse(path)
if err != nil {
// should not happen, since we are trying to access known / hardcoded sockets
return false
}
return fileInfo.Mode()&os.ModeSocket != 0
c, err := net.Dial(u.Scheme, u.Path)
if err != nil {
return false
}
defer c.Close()
return true
}

View File

@ -17,20 +17,29 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
package util
package runtime
import (
"net/url"
winio "github.com/Microsoft/go-winio"
)
const (
dockerSocket = "//./pipe/docker_engine" // The Docker socket is not CRI compatible
containerdSocket = "//./pipe/containerd-containerd" // Proposed containerd named pipe for Windows
dockerSocket = "npipe:////./pipe/docker_engine" // The Docker socket is not CRI compatible
containerdSocket = "npipe:////./pipe/containerd-containerd" // Proposed containerd named pipe for Windows
)
// isExistingSocket checks if path exists and is domain socket
func isExistingSocket(path string) bool {
_, err := winio.DialPipe(path, nil)
u, err := url.Parse(path)
if err != nil {
// should not happen, since we are trying to access known / hardcoded sockets
return false
}
// the dial path must be without "npipe://"
_, err = winio.DialPipe(u.Path, nil)
if err != nil {
return false
}