From 51a0daae4f58d76cf75a739f767ca5b4e8f66bd3 Mon Sep 17 00:00:00 2001 From: Leela Venkaiah G Date: Mon, 29 Jan 2024 12:42:36 +0530 Subject: [PATCH 1/7] controllers: pick next best version of csi images Signed-off-by: Leela Venkaiah G --- controllers/clusterversion_controller.go | 2 +- pkg/csi/csi.go | 38 +++++++++++++++++------- 2 files changed, 28 insertions(+), 12 deletions(-) diff --git a/controllers/clusterversion_controller.go b/controllers/clusterversion_controller.go index ab7aef6f..f873e039 100644 --- a/controllers/clusterversion_controller.go +++ b/controllers/clusterversion_controller.go @@ -139,7 +139,7 @@ func (c *ClusterVersionReconciler) Reconcile(ctx context.Context, req ctrl.Reque return ctrl.Result{}, err } - if err := csi.InitializeSidecars(instance.Status.Desired.Version); err != nil { + if err := csi.InitializeSidecars(c.log, instance.Status.Desired.Version); err != nil { c.log.Error(err, "unable to initialize sidecars") return ctrl.Result{}, err } diff --git a/pkg/csi/csi.go b/pkg/csi/csi.go index 41da28d5..b31240fd 100644 --- a/pkg/csi/csi.go +++ b/pkg/csi/csi.go @@ -20,6 +20,7 @@ import ( "fmt" "os" + "github.com/go-logr/logr" "gopkg.in/yaml.v2" "k8s.io/apimachinery/pkg/util/version" ) @@ -43,9 +44,9 @@ type SidecarImages struct { ContainerImages containerImages `yaml:"containerImages"` } -var sidecarImages = new(SidecarImages) +var sidecarImages *SidecarImages -func InitializeSidecars(ver string) error { +func InitializeSidecars(log logr.Logger, ver string) error { // ready yaml files and yaml unmarshal to SidecarImages // and set to csiSidecarImages si := []SidecarImages{} @@ -58,19 +59,34 @@ func InitializeSidecars(ver string) error { return err } - sv := version.MustParseGeneric(ver) - - for _, image := range si { - v := version.MustParseGeneric(image.Version) - if sv.Major() == v.Major() && sv.Minor() == v.Minor() { - sidecarImages = &image - break + pltVersion := version.MustParseGeneric(ver) + + closestMinor := int64(-1) + for idx := range si { + siVersion := version.MustParseGeneric(si[idx].Version) + log.Info("searching for the most compatible CSI image version", "CSI", siVersion, "Platform", pltVersion) + + // only check sidecar image versions that are not higher than platform + if siVersion.Major() == pltVersion.Major() && siVersion.Minor() <= pltVersion.Minor() { + // filter sidecar closest to platform version + if int64(siVersion.Minor()) > closestMinor { + sidecarImages = &si[idx] + closestMinor = int64(siVersion.Minor()) + } + if closestMinor == int64(pltVersion.Minor()) { // exact match and early exit + break + } + } else { + log.Info("skipping sidecar images: version greater than platform version") } } - if sidecarImages.Version == "" { - return fmt.Errorf("failed to find container details for %v version in %v", sv.String(), sidecarImages) + if sidecarImages == nil { + // happens only if all sidecars image versions are greater than platform + return fmt.Errorf("failed to find container details suitable for %v platform version", pltVersion) } + log.Info("selected sidecar images", "version", sidecarImages.Version) + return nil } From c89998f47a63caa299c539575a3a1a7df665540b Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Wed, 7 Feb 2024 14:44:39 +0100 Subject: [PATCH 2/7] ci: add OWNERS to the repo For the openshift-ci to merge a PR it expect approvals from the user(s) in the OWNER file, This PR adds the required OWNER file. Signed-off-by: Madhu Rajanna --- OWNERS | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 OWNERS diff --git a/OWNERS b/OWNERS new file mode 100644 index 00000000..f37f55d9 --- /dev/null +++ b/OWNERS @@ -0,0 +1,8 @@ +# approval == this is a good idea /approve +approvers: + - Madhu-1 + - nb-ohad +# review == this code is good /lgtm +reviewers: + - Madhu-1 + - nb-ohad From 4a52d27120e9507bcc4125ff5f27317fe9eb6116 Mon Sep 17 00:00:00 2001 From: Rewant Soni Date: Thu, 8 Feb 2024 11:28:18 +0530 Subject: [PATCH 3/7] makefile: update the controller gen version: Signed-off-by: Rewant Soni --- .../crd/bases/ocs.openshift.io_storageclassclaims.yaml | 9 +-------- config/crd/bases/ocs.openshift.io_storageclients.yaml | 9 +-------- config/rbac/role.yaml | 1 - hack/make-bundle-vars.mk | 2 +- hack/make-tools.mk | 2 +- 5 files changed, 4 insertions(+), 19 deletions(-) diff --git a/config/crd/bases/ocs.openshift.io_storageclassclaims.yaml b/config/crd/bases/ocs.openshift.io_storageclassclaims.yaml index 85a6bf87..c3199f5a 100644 --- a/config/crd/bases/ocs.openshift.io_storageclassclaims.yaml +++ b/config/crd/bases/ocs.openshift.io_storageclassclaims.yaml @@ -1,10 +1,9 @@ - --- apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.4.1 + controller-gen.kubebuilder.io/version: v0.9.2 creationTimestamp: null name: storageclassclaims.ocs.openshift.io spec: @@ -90,9 +89,3 @@ spec: storage: true subresources: status: {} -status: - acceptedNames: - kind: "" - plural: "" - conditions: [] - storedVersions: [] diff --git a/config/crd/bases/ocs.openshift.io_storageclients.yaml b/config/crd/bases/ocs.openshift.io_storageclients.yaml index 1eaf0814..766ae368 100644 --- a/config/crd/bases/ocs.openshift.io_storageclients.yaml +++ b/config/crd/bases/ocs.openshift.io_storageclients.yaml @@ -1,10 +1,9 @@ - --- apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.4.1 + controller-gen.kubebuilder.io/version: v0.9.2 creationTimestamp: null name: storageclients.ocs.openshift.io spec: @@ -70,9 +69,3 @@ spec: storage: true subresources: status: {} -status: - acceptedNames: - kind: "" - plural: "" - conditions: [] - storedVersions: [] diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index ddf9f690..1eb7a3a7 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -1,4 +1,3 @@ - --- apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole diff --git a/hack/make-bundle-vars.mk b/hack/make-bundle-vars.mk index 7e652e89..f415a81a 100644 --- a/hack/make-bundle-vars.mk +++ b/hack/make-bundle-vars.mk @@ -70,7 +70,7 @@ CSI_ADDONS_BUNDLE_IMG ?= $(CSI_ADDONS_IMAGE_REGISTRY)/$(CSI_ADDONS_REGISTRY_NAME CATALOG_IMG ?= $(IMAGE_REGISTRY)/$(REGISTRY_NAMESPACE)/$(CATALOG_IMAGE_NAME):$(IMAGE_TAG) # Produce CRDs that work back to Kubernetes 1.11 (no version conversion) -CRD_OPTIONS ?= "crd:trivialVersions=true,preserveUnknownFields=false" +CRD_OPTIONS ?= "crd:generateEmbeddedObjectMeta=true" # A comma-separated list of bundle images (e.g. make catalog-build BUNDLE_IMGS=example.com/operator-bundle:v0.1.0,example.com/operator-bundle:v0.2.0). # These images MUST exist in a registry and be pull-able. diff --git a/hack/make-tools.mk b/hack/make-tools.mk index 2e29c1e5..7ced7c87 100644 --- a/hack/make-tools.mk +++ b/hack/make-tools.mk @@ -10,7 +10,7 @@ endef CONTROLLER_GEN = $(BIN_DIR)/controller-gen controller-gen: ## Download controller-gen locally if necessary. - $(call go-get-tool,$(CONTROLLER_GEN),sigs.k8s.io/controller-tools/cmd/controller-gen@v0.4.1) + $(call go-get-tool,$(CONTROLLER_GEN),sigs.k8s.io/controller-tools/cmd/controller-gen@v0.9.2) KUSTOMIZE = $(BIN_DIR)/kustomize kustomize: ## Download kustomize locally if necessary. From a1f7d600dd632053dd77adbd25de265a2b62cd46 Mon Sep 17 00:00:00 2001 From: Leela Venkaiah G Date: Wed, 14 Feb 2024 15:54:25 +0530 Subject: [PATCH 4/7] controllers: use blank identifier for unused param Signed-off-by: Leela Venkaiah G --- controllers/clusterversion_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controllers/clusterversion_controller.go b/controllers/clusterversion_controller.go index f873e039..eb219603 100644 --- a/controllers/clusterversion_controller.go +++ b/controllers/clusterversion_controller.go @@ -92,7 +92,7 @@ func (c *ClusterVersionReconciler) SetupWithManager(mgr ctrl.Manager) error { ) // Reconcile the ClusterVersion object when the operator config map is updated enqueueClusterVersionRequest := handler.EnqueueRequestsFromMapFunc( - func(_ context.Context, client client.Object) []reconcile.Request { + func(_ context.Context, _ client.Object) []reconcile.Request { return []reconcile.Request{{ NamespacedName: types.NamespacedName{ Name: clusterVersionName, From e1473d9d277f77fd47b302b7ce489ab8222cd7ff Mon Sep 17 00:00:00 2001 From: Leela Venkaiah G Date: Wed, 14 Feb 2024 10:01:04 +0530 Subject: [PATCH 5/7] ci: add leelavg to OWNERS files Signed-off-by: Leela Venkaiah G --- OWNERS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/OWNERS b/OWNERS index f37f55d9..36eefe01 100644 --- a/OWNERS +++ b/OWNERS @@ -2,7 +2,9 @@ approvers: - Madhu-1 - nb-ohad + - leelavg # review == this code is good /lgtm reviewers: - Madhu-1 - nb-ohad + - leelavg From 4f9d5e14553ab57e89d5d4fbc2789fd541e0f40e Mon Sep 17 00:00:00 2001 From: Rewant Soni Date: Thu, 8 Feb 2024 11:39:04 +0530 Subject: [PATCH 6/7] api: make storageClassClaim immutable add kubebuilder validations to make storageClassClaim spec immutable Signed-off-by: Rewant Soni --- api/v1alpha1/storageclassclaim_types.go | 4 +++- config/crd/bases/ocs.openshift.io_storageclassclaims.yaml | 5 +++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/api/v1alpha1/storageclassclaim_types.go b/api/v1alpha1/storageclassclaim_types.go index 2563aa7a..e299d411 100644 --- a/api/v1alpha1/storageclassclaim_types.go +++ b/api/v1alpha1/storageclassclaim_types.go @@ -73,7 +73,9 @@ type StorageClassClaim struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` - Spec StorageClassClaimSpec `json:"spec,omitempty"` + //+kubebuilder:validation:Required + //+kubebuilder:validation:XValidation:rule="oldSelf == self",message="spec is immutable" + Spec StorageClassClaimSpec `json:"spec"` Status StorageClassClaimStatus `json:"status,omitempty"` } diff --git a/config/crd/bases/ocs.openshift.io_storageclassclaims.yaml b/config/crd/bases/ocs.openshift.io_storageclassclaims.yaml index c3199f5a..ae1360cc 100644 --- a/config/crd/bases/ocs.openshift.io_storageclassclaims.yaml +++ b/config/crd/bases/ocs.openshift.io_storageclassclaims.yaml @@ -74,6 +74,9 @@ spec: - storageClient - type type: object + x-kubernetes-validations: + - message: spec is immutable + rule: oldSelf == self status: description: StorageClassClaimStatus defines the observed state of StorageClassClaim properties: @@ -84,6 +87,8 @@ spec: type: string type: array type: object + required: + - spec type: object served: true storage: true From 944e1ef4127e2d807be05d6e12fdddd7949aaa12 Mon Sep 17 00:00:00 2001 From: Rewant Soni Date: Tue, 20 Feb 2024 16:03:32 +0530 Subject: [PATCH 7/7] csi: add csi-addon sidecar to the rbd provisioner deployment Signed-off-by: Rewant Soni --- pkg/csi/rbddeployment.go | 8 +++++ pkg/templates/csisidecars.go | 61 ++++++++++++++++++++++++++++++++++++ pkg/templates/defaults.go | 5 +++ 3 files changed, 74 insertions(+) diff --git a/pkg/csi/rbddeployment.go b/pkg/csi/rbddeployment.go index 4cdd64bc..25f1e331 100644 --- a/pkg/csi/rbddeployment.go +++ b/pkg/csi/rbddeployment.go @@ -60,6 +60,12 @@ func GetRBDDeployment(namespace string) *appsv1.Deployment { ) snapshotter.Image = sidecarImages.ContainerImages.SnapshotterImageURL + csiAddons := templates.CSIAddonsContainer.DeepCopy() + csiAddons.Args = append(csiAddons.Args, + fmt.Sprintf("--leader-election-namespace=%s", namespace), + ) + csiAddons.Image = sidecarImages.ContainerImages.CSIADDONSImageURL + return &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ Name: RBDDeploymentName, @@ -84,6 +90,7 @@ func GetRBDDeployment(namespace string) *appsv1.Deployment { *attacher, *resizer, *snapshotter, + *csiAddons, v1.Container{ Name: "csi-rbdplugin", Image: sidecarImages.ContainerImages.CephCSIImageURL, @@ -95,6 +102,7 @@ func GetRBDDeployment(namespace string) *appsv1.Deployment { "--pidlimit=-1", "--type=rbd", "--controllerserver=true", + fmt.Sprintf("--csi-addons-endpoint=%s", templates.DefaultCSIAddonsSocketPath), fmt.Sprintf("--drivername=%s", GetRBDDriverName()), }, Resources: templates.RBDPluginResourceRequirements, diff --git a/pkg/templates/csisidecars.go b/pkg/templates/csisidecars.go index 3afa9222..6b15a1b0 100644 --- a/pkg/templates/csisidecars.go +++ b/pkg/templates/csisidecars.go @@ -143,6 +143,67 @@ var SnapshotterContainer = &corev1.Container{ }, } +var CSIAddonsContainer = &corev1.Container{ + Name: "csi-addons", + Args: []string{ + "--node-id=$(NODE_ID)", + "--v=5", + fmt.Sprintf("--csi-addons-address=%s", DefaultCSIAddonsSocketPath), + fmt.Sprintf("--controller-port=%v", DefaultCSIAddonsContainerPort), + "--pod=$(POD_NAME)", + "--namespace=$(POD_NAMESPACE)", + "--pod-uid=$(POD_UID)", + fmt.Sprintf("--stagingpath=%s", DefaultStagingPath), + }, + Ports: []corev1.ContainerPort{ + { + ContainerPort: DefaultCSIAddonsContainerPort, + }, + }, + EnvFrom: nil, + Env: []corev1.EnvVar{ + { + Name: "NODE_ID", + ValueFrom: &corev1.EnvVarSource{ + FieldRef: &corev1.ObjectFieldSelector{ + FieldPath: "spec.nodeName", + }, + }, + }, + { + Name: "POD_UID", + ValueFrom: &corev1.EnvVarSource{ + FieldRef: &corev1.ObjectFieldSelector{ + FieldPath: "metadata.uid", + }, + }, + }, + { + Name: "POD_NAME", + ValueFrom: &corev1.EnvVarSource{ + FieldRef: &corev1.ObjectFieldSelector{ + FieldPath: "metadata.name", + }, + }, + }, + { + Name: "POD_NAMESPACE", + ValueFrom: &corev1.EnvVarSource{ + FieldRef: &corev1.ObjectFieldSelector{ + FieldPath: "metadata.namespace", + }, + }, + }, + }, + VolumeMounts: []corev1.VolumeMount{ + { + Name: "socket-dir", + MountPath: DefaultSocketDir, + }, + }, + ImagePullPolicy: corev1.PullIfNotPresent, +} + var DriverRegistrar = &corev1.Container{ Name: "csi-driver-registrar", ImagePullPolicy: corev1.PullIfNotPresent, diff --git a/pkg/templates/defaults.go b/pkg/templates/defaults.go index b051fb33..ccfbce7c 100644 --- a/pkg/templates/defaults.go +++ b/pkg/templates/defaults.go @@ -21,9 +21,14 @@ const ( DefaultKubeletDirPath = "/var/lib/kubelet" DefaultProvisionerSocketPath = "unix:///csi/csi-provisioner.sock" DefaultPluginSocketPath = "unix:///csi/csi.sock" + DefaultCSIAddonsSocketPath = "unix:///csi/csi-addons.sock" DefaultSocketDir = "/csi" + DefaultStagingPath = "/var/lib/kubelet/plugins/kubernetes.io/csi/" // configmap names MonConfigMapName = "ceph-csi-configs" EncryptionConfigMapName = "ceph-csi-kms-config" + + // default port numbers + DefaultCSIAddonsContainerPort = int32(9070) )