From ea2b912265c4187dada70e540eb71edc25e02534 Mon Sep 17 00:00:00 2001 From: Carl Montanari Date: Sat, 9 Dec 2023 12:06:57 -0800 Subject: [PATCH 1/3] chore: tidy makefile comment/doc --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 2d9d7b00..aff128e2 100644 --- a/Makefile +++ b/Makefile @@ -14,7 +14,7 @@ fmt: ## Run formatters goimports -w . golines -w . -lint: fmt ## Run linters; runs with GOOS env var for linting on darwin +lint: fmt ## Run linters golangci-lint run helm lint --quiet charts/clabernetes helm lint --quiet charts/clicker From 1293c5dc2c8723c856a16b848135cb0d39448bf0 Mon Sep 17 00:00:00 2001 From: Carl Montanari Date: Sat, 9 Dec 2023 12:07:35 -0800 Subject: [PATCH 2/3] feat: add criSock/criKind overrides to global config --- apis/v1alpha1/configspec.go | 16 +++ .../clabernetes.containerlab.dev_configs.yaml | 21 +++ .../clabernetes.containerlab.dev_configs.yaml | 21 +++ charts/clabernetes/values.yaml | 10 ++ config/fake.go | 8 ++ config/get.go | 14 ++ config/manager.go | 4 + controllers/topology/deployment.go | 125 +++++++++++------- generated/openapi/openapi_generated.go | 14 ++ 9 files changed, 185 insertions(+), 48 deletions(-) diff --git a/apis/v1alpha1/configspec.go b/apis/v1alpha1/configspec.go index e92182f9..50894f19 100644 --- a/apis/v1alpha1/configspec.go +++ b/apis/v1alpha1/configspec.go @@ -75,4 +75,20 @@ type ConfigImagePull struct { // +kubebuilder:validation:Enum=auto;always;never // +optional PullThroughOverride string `json:"pullThroughOverride,omitempty"` + // CRISockOverride allows for overriding the path of the CRI sock that is mounted in the + // launcher pods (if/when image pull through mode is auto or always). This can be useful if, + // for example, the CRI sock is in a "non-standard" location like K3s which puts the containerd + // sock at `/run/k3s/containerd/containerd.sock` rather than the "normal" (whatever that means) + // location of `/run/containerd/containerd.sock`. The value must end with "containerd.sock" for + // now, in the future maybe crio support will be added. + // +kubebuilder:validation:Pattern=(.*containerd\.sock) + // +optional + CRISockOverride string `json:"criSockOverride,omitempty"` + // CRIKindOverride allows for overriding the auto discovered cri flavor of the cluster -- this + // may be useful if we fail to parse the cri kind for some reason, or in mixed cri flavor + // clusters -- however in the latter case, make sure that if you are using image pull through + // that clabernetes workloads are only run on the nodes of the cri kind specified here! + // +kubebuilder:validation:Enum=containerd + // +optional + CRIKindOverride string `json:"criKindOverride,omitempty"` } diff --git a/assets/crd/clabernetes.containerlab.dev_configs.yaml b/assets/crd/clabernetes.containerlab.dev_configs.yaml index 54aacdd3..ccdc8180 100644 --- a/assets/crd/clabernetes.containerlab.dev_configs.yaml +++ b/assets/crd/clabernetes.containerlab.dev_configs.yaml @@ -205,6 +205,27 @@ spec: description: ImagePull holds configurations relevant to how clabernetes launcher pods handle pulling images. properties: + criKindOverride: + description: CRIKindOverride allows for overriding the auto discovered + cri flavor of the cluster -- this may be useful if we fail to + parse the cri kind for some reason, or in mixed cri flavor clusters + -- however in the latter case, make sure that if you are using + image pull through that clabernetes workloads are only run on + the nodes of the cri kind specified here! + enum: + - containerd + type: string + criSockOverride: + description: CRISockOverride allows for overriding the path of + the CRI sock that is mounted in the launcher pods (if/when image + pull through mode is auto or always). This can be useful if, + for example, the CRI sock is in a "non-standard" location like + K3s which puts the containerd sock at `/run/k3s/containerd/containerd.sock` + rather than the "normal" (whatever that means) location of `/run/containerd/containerd.sock`. + The value must end with "containerd.sock" for now, in the future + maybe crio support will be added. + pattern: (.*containerd\.sock) + type: string pullThroughOverride: description: PullThroughOverride allows for overriding the image pull through mode for this particular topology. diff --git a/charts/clabernetes/crds/clabernetes.containerlab.dev_configs.yaml b/charts/clabernetes/crds/clabernetes.containerlab.dev_configs.yaml index 54aacdd3..ccdc8180 100644 --- a/charts/clabernetes/crds/clabernetes.containerlab.dev_configs.yaml +++ b/charts/clabernetes/crds/clabernetes.containerlab.dev_configs.yaml @@ -205,6 +205,27 @@ spec: description: ImagePull holds configurations relevant to how clabernetes launcher pods handle pulling images. properties: + criKindOverride: + description: CRIKindOverride allows for overriding the auto discovered + cri flavor of the cluster -- this may be useful if we fail to + parse the cri kind for some reason, or in mixed cri flavor clusters + -- however in the latter case, make sure that if you are using + image pull through that clabernetes workloads are only run on + the nodes of the cri kind specified here! + enum: + - containerd + type: string + criSockOverride: + description: CRISockOverride allows for overriding the path of + the CRI sock that is mounted in the launcher pods (if/when image + pull through mode is auto or always). This can be useful if, + for example, the CRI sock is in a "non-standard" location like + K3s which puts the containerd sock at `/run/k3s/containerd/containerd.sock` + rather than the "normal" (whatever that means) location of `/run/containerd/containerd.sock`. + The value must end with "containerd.sock" for now, in the future + maybe crio support will be added. + pattern: (.*containerd\.sock) + type: string pullThroughOverride: description: PullThroughOverride allows for overriding the image pull through mode for this particular topology. diff --git a/charts/clabernetes/values.yaml b/charts/clabernetes/values.yaml index 0ee2439a..c8199172 100644 --- a/charts/clabernetes/values.yaml +++ b/charts/clabernetes/values.yaml @@ -88,6 +88,16 @@ globalConfig: # ever pull via the docker daemon in the launcher pod itself (bypassing the cluster). Note that # "pull through mode" currently only supports containerd as a CRI. imagePullThroughMode: auto + # criSockOverride allows for overriding the path of the CRI sock that is mounted in the + # launcher pods (if/when image pull through mode is auto or always). This can be useful if, + # for example, the CRI sock is in a "non-standard" location like K3s which puts the containerd + # sock at `/run/k3s/containerd/containerd.sock` rather than the "normal" (whatever that means) + # location of `/run/containerd/containerd.sock`. + # criSockOverride: "" + # criKindOverride allows for overriding teh auto discovered cri kind. Probably/hopefully this + # won't be needed often, but could come in handy in multi-cri clusters or if nodes for some + # reason do not properly report their cri flavor (or we incorrectly parse it?!) + # criKindOverride: "" deployment: # resourcesDefault hold the default resources to apply to clabernetes launcher pods. diff --git a/config/fake.go b/config/fake.go index 27bc3b61..9a773918 100644 --- a/config/fake.go +++ b/config/fake.go @@ -58,6 +58,14 @@ func (f fakeManager) GetLauncherImage() string { return "ghcr.io/srl-labs/clabernetes/clabernetes-launcher:latest" } +func (f fakeManager) GetImagePullCriSockOverride() string { + return "" +} + +func (f fakeManager) GetImagePullCriKindOverride() string { + return "" +} + func (f fakeManager) GetLauncherImagePullPolicy() string { return clabernetesconstants.KubernetesImagePullIfNotPresent } diff --git a/config/get.go b/config/get.go index 6e75938f..784316f3 100644 --- a/config/get.go +++ b/config/get.go @@ -92,6 +92,20 @@ func (m *manager) GetImagePullThroughMode() string { return m.config.ImagePull.PullThroughOverride } +func (m *manager) GetImagePullCriSockOverride() string { + m.lock.RLock() + defer m.lock.RUnlock() + + return m.config.ImagePull.CRISockOverride +} + +func (m *manager) GetImagePullCriKindOverride() string { + m.lock.RLock() + defer m.lock.RUnlock() + + return m.config.ImagePull.CRIKindOverride +} + func (m *manager) GetLauncherImage() string { m.lock.RLock() defer m.lock.RUnlock() diff --git a/config/manager.go b/config/manager.go index 12b56f8d..e8db05b2 100644 --- a/config/manager.go +++ b/config/manager.go @@ -130,6 +130,10 @@ type Manager interface { GetInClusterDNSSuffix() string // GetImagePullThroughMode returns the image pull through mode in the global config. GetImagePullThroughMode() string + // GetImagePullCriSockOverride returns the cri sock path override. + GetImagePullCriSockOverride() string + // GetImagePullCriKindOverride returns the cri kind override. + GetImagePullCriKindOverride() string // GetLauncherImage returns the global default launcher image. GetLauncherImage() string // GetLauncherImagePullPolicy returns the global default launcher image pull policy. diff --git a/controllers/topology/deployment.go b/controllers/topology/deployment.go index c91ccd6b..592fb101 100644 --- a/controllers/topology/deployment.go +++ b/controllers/topology/deployment.go @@ -2,6 +2,7 @@ package topology import ( "fmt" + "path/filepath" "reflect" "strings" @@ -188,57 +189,35 @@ func (r *DeploymentReconciler) renderDeploymentVolumes( volumeMountsFromCommonSpec := make([]k8scorev1.VolumeMount, 0) - // if we have containerd cri *and* pull through mode is auto or always, we need to mount the - // containerd sock - if r.configManagerGetter(). - GetImagePullThroughMode() != - clabernetesconstants.ImagePullThroughModeNever && - owningTopology.Spec.ImagePull.PullThroughOverride != clabernetesconstants.ImagePullThroughModeNever { //nolint:lll - var path string + criPath, criSubPath := r.renderDeploymentVolumesGetCRISockPath(owningTopology) - var subPath string - - switch r.criKind { - case clabernetesconstants.KubernetesCRIContainerd: - path = clabernetesconstants.KubernetesCRISockContainerdPath - - subPath = clabernetesconstants.KubernetesCRISockContainerd - default: - r.log.Warnf( - "image pull through mode is auto or always but cri kind is not containerd!"+ - " got cri kind %q", - r.criKind, - ) - } - - if path != "" && subPath != "" { - volumes = append( - volumes, - k8scorev1.Volume{ - Name: "cri-sock", - VolumeSource: k8scorev1.VolumeSource{ - HostPath: &k8scorev1.HostPathVolumeSource{ - Path: path, - Type: clabernetesutil.ToPointer(k8scorev1.HostPathType("")), - }, + if criPath != "" && criSubPath != "" { + volumes = append( + volumes, + k8scorev1.Volume{ + Name: "cri-sock", + VolumeSource: k8scorev1.VolumeSource{ + HostPath: &k8scorev1.HostPathVolumeSource{ + Path: criPath, + Type: clabernetesutil.ToPointer(k8scorev1.HostPathType("")), }, }, - ) + }, + ) - volumeMountsFromCommonSpec = append( - volumeMountsFromCommonSpec, - k8scorev1.VolumeMount{ - Name: "cri-sock", - ReadOnly: true, - MountPath: fmt.Sprintf( - "%s/%s", - clabernetesconstants.LauncherCRISockPath, - subPath, - ), - SubPath: subPath, - }, - ) - } + volumeMountsFromCommonSpec = append( + volumeMountsFromCommonSpec, + k8scorev1.VolumeMount{ + Name: "cri-sock", + ReadOnly: true, + MountPath: fmt.Sprintf( + "%s/%s", + clabernetesconstants.LauncherCRISockPath, + criSubPath, + ), + SubPath: criSubPath, + }, + ) } volumesFromConfigMaps := make([]clabernetesapisv1alpha1.FileFromConfigMap, 0) @@ -286,6 +265,51 @@ func (r *DeploymentReconciler) renderDeploymentVolumes( return volumeMountsFromCommonSpec } +func (r *DeploymentReconciler) renderDeploymentVolumesGetCRISockPath( + owningTopology *clabernetesapisv1alpha1.Topology, +) (path, subPath string) { + if owningTopology.Spec.ImagePull.PullThroughOverride == clabernetesconstants.ImagePullThroughModeNever { //nolint:lll + // obviously the topology is set to *never*, so nothing to do... + return path, subPath + } + + if owningTopology.Spec.ImagePull.PullThroughOverride == "" && r.configManagerGetter(). + GetImagePullThroughMode() == clabernetesconstants.ImagePullThroughModeNever { + // our specific topology is setting is unset, so we default to the global value, if that + // is never then we are obviously done here + return path, subPath + } + + criSockOverrideFullPath := r.configManagerGetter().GetImagePullCriSockOverride() + if criSockOverrideFullPath != "" { + path, subPath = filepath.Split(criSockOverrideFullPath) + + if path == "" { + r.log.Warn( + "image pull cri path override is set, but failed to parse path/subpath," + + " will skip mounting cri sock", + ) + + return path, subPath + } + } else { + switch r.criKind { + case clabernetesconstants.KubernetesCRIContainerd: + path = clabernetesconstants.KubernetesCRISockContainerdPath + + subPath = clabernetesconstants.KubernetesCRISockContainerd + default: + r.log.Warnf( + "image pull through mode is auto or always but cri kind is not containerd!"+ + " got cri kind %q", + r.criKind, + ) + } + } + + return path, subPath +} + func (r *DeploymentReconciler) renderDeploymentContainer( deployment *k8sappsv1.Deployment, nodeName, @@ -368,6 +392,11 @@ func (r *DeploymentReconciler) renderDeploymentContainerEnv( imagePullThroughMode = r.configManagerGetter().GetImagePullThroughMode() } + criKind := r.configManagerGetter().GetImagePullCriKindOverride() + if criKind == "" { + criKind = r.criKind + } + envs := []k8scorev1.EnvVar{ { Name: clabernetesconstants.NodeNameEnv, @@ -406,7 +435,7 @@ func (r *DeploymentReconciler) renderDeploymentContainerEnv( }, { Name: clabernetesconstants.LauncherCRIKindEnv, - Value: r.criKind, + Value: criKind, }, { Name: clabernetesconstants.LauncherImagePullThroughModeEnv, diff --git a/generated/openapi/openapi_generated.go b/generated/openapi/openapi_generated.go index e7cf6e94..df20c399 100644 --- a/generated/openapi/openapi_generated.go +++ b/generated/openapi/openapi_generated.go @@ -255,6 +255,20 @@ func schema_srl_labs_clabernetes_apis_v1alpha1_ConfigImagePull( Format: "", }, }, + "criSockOverride": { + SchemaProps: spec.SchemaProps{ + Description: "CRISockOverride allows for overriding the path of the CRI sock that is mounted in the launcher pods (if/when image pull through mode is auto or always). This can be useful if, for example, the CRI sock is in a \"non-standard\" location like K3s which puts the containerd sock at `/run/k3s/containerd/containerd.sock` rather than the \"normal\" (whatever that means) location of `/run/containerd/containerd.sock`. The value must end with \"containerd.sock\" for now, in the future maybe crio support will be added.", + Type: []string{"string"}, + Format: "", + }, + }, + "criKindOverride": { + SchemaProps: spec.SchemaProps{ + Description: "CRIKindOverride allows for overriding the auto discovered cri flavor of the cluster -- this may be useful if we fail to parse the cri kind for some reason, or in mixed cri flavor clusters -- however in the latter case, make sure that if you are using image pull through that clabernetes workloads are only run on the nodes of the cri kind specified here!", + Type: []string{"string"}, + Format: "", + }, + }, }, }, }, From e4e84a15c04144aa0ce4317ccc87f2bf08048120 Mon Sep 17 00:00:00 2001 From: Carl Montanari Date: Sat, 9 Dec 2023 12:53:08 -0800 Subject: [PATCH 3/3] feat: bootstrap config handle cri sock/kind global config --- charts/clabernetes/templates/configmap.yaml | 6 ++++++ config/bootstrap.go | 22 +++++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/charts/clabernetes/templates/configmap.yaml b/charts/clabernetes/templates/configmap.yaml index 94305f00..8458559b 100644 --- a/charts/clabernetes/templates/configmap.yaml +++ b/charts/clabernetes/templates/configmap.yaml @@ -41,4 +41,10 @@ data: launcherImage: {{ .Values.globalConfig.deployment.launcherImage }} launcherImagePullPolicy: {{ .Values.globalConfig.deployment.launcherImagePullPolicy }} launcherLogLevel: {{ .Values.globalConfig.deployment.launcherLogLevel }} + {{- if .Values.globalConfig.imagePull.criSockOverride }} + criSockOverride: {{ .Values.globalConfig.imagePull.criSockOverride }} + {{- end }} + {{- if .Values.globalConfig.imagePull.criKindOverride }} + criKindOverride: {{ .Values.globalConfig.imagePull.criKindOverride }} + {{- end }} {{- end }} \ No newline at end of file diff --git a/config/bootstrap.go b/config/bootstrap.go index 371c0dad..a06b4cdc 100644 --- a/config/bootstrap.go +++ b/config/bootstrap.go @@ -27,6 +27,8 @@ type bootstrapConfig struct { launcherImage string launcherImagePullPolicy string launcherLogLevel string + criSockOverride string + criKindOverride string } func bootstrapFromConfigMap( //nolint:gocyclo,funlen @@ -119,6 +121,16 @@ func bootstrapFromConfigMap( //nolint:gocyclo,funlen bc.launcherLogLevel = launcherLogLevel } + criSockOverride, criSockOverrideOk := inMap["criSockOverride"] + if criSockOverrideOk { + bc.criSockOverride = criSockOverride + } + + criKindOverride, criKindOverrideOk := inMap["criKindOverride"] + if criKindOverrideOk { + bc.criKindOverride = criKindOverride + } + var err error if len(outErrors) > 0 { @@ -211,6 +223,14 @@ func mergeFromBootstrapConfigMerge( if config.Spec.Deployment.LauncherLogLevel == "" { config.Spec.Deployment.LauncherLogLevel = bootstrap.launcherLogLevel } + + if config.Spec.ImagePull.CRISockOverride == "" { + config.Spec.ImagePull.CRISockOverride = bootstrap.criSockOverride + } + + if config.Spec.ImagePull.CRIKindOverride == "" { + config.Spec.ImagePull.CRIKindOverride = bootstrap.criKindOverride + } } func mergeFromBootstrapConfigReplace( @@ -225,6 +245,8 @@ func mergeFromBootstrapConfigReplace( InClusterDNSSuffix: bootstrap.inClusterDNSSuffix, ImagePull: clabernetesapisv1alpha1.ConfigImagePull{ PullThroughOverride: bootstrap.imagePullThroughMode, + CRISockOverride: bootstrap.criSockOverride, + CRIKindOverride: bootstrap.criKindOverride, }, Deployment: clabernetesapisv1alpha1.ConfigDeployment{ ResourcesDefault: bootstrap.resourcesDefault,