search

janus-idp / operator

Deprecated - Operator for Backstage, based on the Operator SDK framework - see https://github.com/redhat-developer/rhdh-operator
https://github.com/redhat-developer/rhdh-operator
Apache License 2.0
15 stars 15 forks source link

[1.2.x] Allow configuring the storage class name for PVCs used by both DB and app #392

Closed rm3l closed 4 months ago

rm3l commented 4 months ago

Description

This PR adds 2 new fields to the CRD:

They allow configuring the storage classes to be used by the PVCs created by the Operator, i.e., the ephemeral PVC created for the dynamic-plugins-root volume, as well as the PVC used by the local database StatefulSet.

https://github.com/janus-idp/operator/pull/388 intended to provide a more generic way to handle this (along with supporting customizing other properties like labels, annotations, resources, sidecar containers, ...), but it currently does not apply to the local database.

Given that https://issues.redhat.com/browse/RHIDP-2705 is a current a blocker for the upcoming 1.2.2 release, this PR is more focused, covering only the storage class use case with the current v1alpha1 API version.

https://github.com/janus-idp/operator/pull/388 will need to be updated later for 1.3.

Preview in the OCP console Screenshot from 2024-07-01 12-56-19

Which issue(s) does this PR fix or relate to

PR acceptance criteria

How to test changes / Special notes to the reviewer

(Tested on a ClusterBot cluster, but feel free to use any storage class on your particular cluster)

cat <<EOF | kubectl apply -f -
apiVersion: rhdh.redhat.com/v1alpha1
kind: Backstage
metadata:
  name: developer-hub
spec:
  application:
    storageClassName: "gp2-csi"
  database:
    storageClassName: "gp3-csi"
EOF

Then run kubectl get pvc and check the storage classes used by the PVCs, e.g.:

$ kubectl get pvc
NAME                                                               STATUS   VOLUME                                     CAPACITY   ACCESS MODES   STORAGECLASS   AGE
backstage-backstage-sample-5dbd785b5c-6h7qt-dynamic-plugins-root   Bound    pvc-06f259dc-6c02-4fbd-a6d1-4a78d40ca9ac   2Gi        RWO            gp2-csi        4m43s
data-backstage-psql-backstage-sample-0                             Bound    pvc-2e44619b-cf1c-4d62-bc4c-fea423537648   1Gi        RWO            gp3-csi        4m43s
openshift-ci[bot] commented 4 months ago

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Once this PR has been reviewed and has the lgtm label, please ask for approval from rm3l. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files: - **[OWNERS](https://github.com/janus-idp/operator/blob/1.2.x/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
github-actions[bot] commented 4 months ago

PR images are available:

  1. https://quay.io/janus-idp/operator:0.2.1-pr-392-0a54885
  2. https://quay.io/janus-idp/operator-bundle:0.2.1-pr-392-0a54885
  3. https://quay.io/janus-idp/operator-catalog:0.2.1-pr-392-0a54885

github-actions[bot] commented 4 months ago

PR images are available:

  1. https://quay.io/janus-idp/operator:0.2.2-pr-392-6c45a3a
  2. https://quay.io/janus-idp/operator-bundle:0.2.2-pr-392-6c45a3a
  3. https://quay.io/janus-idp/operator-catalog:0.2.2-pr-392-6c45a3a

rm3l commented 4 months ago

I do not think new feature, especially related to API change can be a part of patch version (see semver.org).

Instead, to make deployment configuration feature to be available sooner, I would propose to make a 0.3 (1.3) operator release, including #388, ASAP.

Sure, it can be seen as a feature semantically-speaking, but in my understanding, https://issues.redhat.com/browse/RHIDP-2705 has been marked as a blocker for 1.2.2, upon customer request. So it cannot wait for 1.3, I guess.

388 covers too many things right now; so to minimize the risks of introducing breaking changes to 1.2.2, it was concluded that a dedicated PR (addressing only the storage class request) would be created for 1.2.2, hence this PR.

This PR only targets 1.2.x, so only for the upcoming 1.2.2 release, which will leave time to address the review comments on #388 for 1.3 Hope that clarifies.

gazarenkov commented 4 months ago

I do not think new feature, especially related to API change can be a part of patch version (see semver.org). Instead, to make deployment configuration feature to be available sooner, I would propose to make a 0.3 (1.3) operator release, including #388, ASAP.

Sure, it can be seen as a feature semantically-speaking, but in my understanding, https://issues.redhat.com/browse/RHIDP-2705 has been marked as a blocker for 1.2.2, upon customer request. So it cannot wait for 1.3, I guess.

388 covers too many things right now; so to minimize the risks of introducing breaking changes to 1.2.2, it was concluded that a dedicated PR (addressing only the storage class request) would be created for 1.2.2, hence this PR. This PR only targets 1.2.x, so only for the upcoming 1.2.2 release, which will leave time to address the review comments on #388 for 1.3 Hope that clarifies.

No, it does not, sorry :)

rm3l commented 4 months ago

Customer confirmed this is not needed in 1.2.x

/close

openshift-ci[bot] commented 4 months ago

@rm3l: Closed this PR.

In response to [this](https://github.com/janus-idp/operator/pull/392#issuecomment-2223081247): >Customer confirmed this is not needed in 1.2.x > >/close Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.