Deployment configuration and v1alpha2 version #388

gazarenkov · 2024-06-20T07:32:45Z

Description

This PR includes new way to configure Backstage deployment, now (in contrary to raw config) it is possible to add the changes as a and valid "fragment" of appsv1.deployment object inside Backstage CR and those fields will be merged with default (+raw) configuration, like:

apiVersion: rhdh.redhat.com/v1alpha2
kind: Backstage
metadata:
  name: my-backstage
spec:
  deployment:
    patch:
      spec:
        template:
          metadata:
             labels:
                label1: value1
          spec:
            containers:
              - name: backstage-backend
                 resources:
                    requests:
                        cpu: 250m
                        memory: 256Mi
             - name: my-sidecar
                image: my-sidecar
            volumes:
              - ephemeral:
                  volumeClaimTemplate:
                    spec:
                      storageClassName: "my-special-class"
                name: dynamic-plugins-root

Also, since this addition causes changes in the API, new API version v1alpha2 was added. So, as for Operator versions 0.3.x and 0.4.x (if we consider 3 months release cadency and 6 months API maintain) it will support as minimum both v1alpha1 and v1alpha2 (which become a storage version). After upgrading Operator to 0.3.x the following behaviour expected:

existed v1alpha1 CRs are working and shown as v1alpha2
both v1alpha1 and v1alpha2 CRs can be added/updated, all shown as v1alpha2

NOTE: v1alpha2 is backward compatible with v1alpha1 w/o data conversion

Openshift NOTE: deployment.patch field won't show up in the "Form view" when creating a CR from the OCP Web. To be able to edit it with ODC use "YAML view".

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

#352

PR acceptance criteria

Tests
Documentation, become part of Configuration documentation #129
If the bundle manifests have been updated, make sure to review the rhdh-operator.csv.yaml file accordingly

How to test changes / Special notes to the reviewer

Create v1alpha2 Backstage resource as mentioned above
Create resource on 0.2.0 version, then upgrade operator and try to update old and create new resource

Signed-off-by: gazarenkov <[email protected]>

# Conflicts: # bundle/manifests/backstage-operator.clusterserviceversion.yaml # config/manifests/bases/backstage-operator.clusterserviceversion.yaml # integration_tests/db_test.go # integration_tests/default-config_test.go # pkg/model/secretfiles_test.go

Signed-off-by: gazarenkov <[email protected]>

openshift-ci · 2024-06-20T07:32:52Z

[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 gazarenkov. 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

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Co-authored-by: gazarenkov <[email protected]>

github-actions · 2024-06-20T07:33:52Z

⚠️ Files changed in bundle generation!

Those changes to the operator bundle manifests should have been pushed automatically to your PR branch.
You might also need to manually update the .rhdh/bundle/manifests/rhdh-operator.csv.yaml CSV file accordingly.

github-actions · 2024-06-20T07:39:11Z

PR images are available:

api/v1alpha2/backstage_types.go

.rhdh/bundle/manifests/rhdh-operator.csv.yaml

rm3l · 2024-06-21T12:23:03Z

api/v1alpha2/backstage_types.go

+	// Defaults to 1.
+	// +optional
+	//+kubebuilder:default=1
+	Replicas *int32 `json:"replicas,omitempty"`


I guess we need to remove Replicas, Image, and ImagePullSecrets fields from this version, since they are marked as deprecated in v1alpha1 (and the goal is to stop using them)? And the controller would need to manage both versions.
Otherwise, I don't see the point of having a new API Version if we are just adding a new spec.deployment field to the CR (in a backward-compatible manner). We could just add this new field without changing the API version, IMO.
Reference: https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api_changes.md#changing-the-api

Also, what would be the behavior if for example both spec.application.replicas and spec.deployment.spec.replicas are set in the CR?

I'd agree with you in case if do not have GA obligations (indeed "alpha" API allows to do pretty much "whatever you want"). It is definitely simpler way for development. But, it is what it is, and we have to support older API for some time (6 mo).
Anyway, even if we have kind of choice now, I'd consider it as a good exercise for the case when we declare the API as more stable but need to change it, so, I see no reason why not to change the API version if the API is changed :).

Also, what would be the behavior if for example both spec.application.replicas and spec.deployment.spec.replicas are set in the CR?

It is a good point, I think it should be fixed, so new API always wins.

I'd agree with you in case if do not have GA obligations (indeed "alpha" API allows to do pretty much "whatever you want"). It is definitely simpler way for development. But, it is what it is, and we have to support older API for some time (6 mo).

Well, I meant, to remove the deprecated fields from the new v1alpha2 version (not v1alpha1, which I agree still needs to be supported). Otherwise, my point was to say that if we want to keep these deprecated fields in v1alpha2, then maybe v1alpha2 is not needed at all, and the new fields could simply be added to v1alpha1.

Not sure I understand how to support those fields if we remove them. Thoughts?

As documented in https://sdk.operatorframework.io/docs/best-practices/best-practices/, I guess one approach could be to implement CRD conversion here, to automatically convert older v1alpha1 CRs to this new v1alpha2.
But, even if it may be an interesting exercise, we should still be somewhat thoughtful about how we approach this. I still think that we could add the new fields you are suggesting here to the existing v1alpha1; IMO adding new fields to the CRD is a compatible change that should not require a new API version.
We could just document the deprecated ones accordingly.

As you could read in the NOTE above: "v1alpha2 is backward compatible with v1alpha1 w/o data conversion"

Not sure what exactly you mean by "been thoughtful", my point is to make it supportable which mean the API version should unequivocally identify the state. So if "v1alpha1" may mean several different (even compatible) combinations of config is not appropriate for supportable (GA) application.

rm3l · 2024-06-21T12:24:06Z

api/v1alpha2/backstage_types.go

+	// Custom image to use in all containers (including Init Containers).
+	// It is your responsibility to make sure the image is from trusted sources and has been validated for security compliance
+	// +optional
+	Image *string `json:"image,omitempty"`


See previous comment..

rm3l · 2024-06-21T12:24:12Z

api/v1alpha2/backstage_types.go

+
+	// Image Pull Secrets to use in all containers (including Init Containers)
+	// +optional
+	ImagePullSecrets []string `json:"imagePullSecrets,omitempty"`


See previous comment..

pkg/model/deployment.go

rm3l · 2024-06-21T14:17:02Z

api/v1alpha2/backstage_types.go

+	// Set the Deployment's metadata and|or spec fields you want to override or add.
+	// Optional.
+	// +kubebuilder:pruning:PreserveUnknownFields
+	Deployment *apiextensionsv1.JSON `json:"deployment,omitempty"`


It looks like this new field won't show up in the "Form view" when creating a CR from the OCP Web Console. I guess because of the field type.

Could you investigate how we could display it there? Maybe as a descriptor in the CSV?

Note that it correctly shows up in the "YAML view" with the help of completion..

I think it is due to field type, it is unstructured per se.

I think we can create an issue for ODC about it, meantime it should be solved by downstream documentation.

added documentation note to PR

pkg/model/db-statefulset.go

# Conflicts: # bundle/manifests/backstage-operator.clusterserviceversion.yaml

Signed-off-by: gazarenkov <[email protected]>

…t_patch # Conflicts: # bundle/manifests/backstage-operator.clusterserviceversion.yaml

Co-authored-by: gazarenkov <[email protected]>

github-actions · 2024-07-08T12:45:20Z

⚠️ Files changed in bundle generation!

Those changes to the operator bundle manifests should have been pushed automatically to your PR branch.
You might also need to manually update the .rhdh/bundle/manifests/rhdh-operator.csv.yaml CSV file accordingly.

github-actions · 2024-07-08T12:50:26Z

PR images are available:

Signed-off-by: gazarenkov <[email protected]>

…t_patch

github-actions · 2024-07-08T14:46:13Z

PR images are available:

openshift-merge-robot · 2024-07-12T22:35:53Z

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

gazarenkov added 6 commits June 18, 2024 13:46

kyaml and tests

c9238e0

Signed-off-by: gazarenkov <[email protected]>

added v1alpha2

fcdc6c2

Signed-off-by: gazarenkov <[email protected]>

deprecation message

b7c6db5

Signed-off-by: gazarenkov <[email protected]>

fix spec.deployment test

38b5d7e

Signed-off-by: gazarenkov <[email protected]>

.rhdh/csv modified

e307937

Signed-off-by: gazarenkov <[email protected]>

gazarenkov requested review from kadel and rm3l June 20, 2024 07:32

gazarenkov temporarily deployed to internal June 20, 2024 07:32 — with GitHub Actions Inactive

openshift-ci bot requested a review from nickboldt June 20, 2024 07:32

gazarenkov mentioned this pull request Jun 20, 2024

Improve Backstage CRD to simplify k8s runtime customization #352

Open

Regenerate bundle manifests

e2809e5

Co-authored-by: gazarenkov <[email protected]>

kadel reviewed Jun 20, 2024

View reviewed changes

api/v1alpha2/backstage_types.go Outdated Show resolved Hide resolved

rm3l reviewed Jun 21, 2024

View reviewed changes

rm3l reviewed Jun 29, 2024

View reviewed changes

pkg/model/db-statefulset.go Outdated Show resolved Hide resolved

rm3l mentioned this pull request Jul 1, 2024

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

Closed

3 tasks

openshift-merge-robot added the needs-rebase label Jul 4, 2024

gazarenkov added 3 commits July 8, 2024 14:01

Merge remote-tracking branch 'upstream/main' into deployment_patch

3c0b18f

# Conflicts: # bundle/manifests/backstage-operator.clusterserviceversion.yaml

fixes after comments

9733168

Signed-off-by: gazarenkov <[email protected]>

Merge remote-tracking branch 'origin/deployment_patch' into deploymen…

14c1b91

…t_patch # Conflicts: # bundle/manifests/backstage-operator.clusterserviceversion.yaml

gazarenkov temporarily deployed to internal July 8, 2024 12:44 — with GitHub Actions Inactive

openshift-merge-robot removed the needs-rebase label Jul 8, 2024

Regenerate bundle manifests

ca05c82

Co-authored-by: gazarenkov <[email protected]>

gazarenkov added 2 commits July 8, 2024 17:35

fixes after comments

496bb19

Signed-off-by: gazarenkov <[email protected]>

Merge remote-tracking branch 'origin/deployment_patch' into deploymen…

a0b14b1

…t_patch

gazarenkov temporarily deployed to internal July 8, 2024 14:40 — with GitHub Actions Inactive

gazarenkov requested review from kadel and rm3l July 8, 2024 14:44

openshift-merge-robot added the needs-rebase label Jul 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deployment configuration and v1alpha2 version #388

Deployment configuration and v1alpha2 version #388

gazarenkov commented Jun 20, 2024 •

edited

Loading

openshift-ci bot commented Jun 20, 2024

github-actions bot commented Jun 20, 2024

github-actions bot commented Jun 20, 2024

rm3l Jun 21, 2024 •

edited

Loading

rm3l Jun 21, 2024

gazarenkov Jul 8, 2024

rm3l Jul 8, 2024

gazarenkov Jul 8, 2024

rm3l Jul 8, 2024

gazarenkov Jul 9, 2024

rm3l Jun 21, 2024

rm3l Jun 21, 2024

rm3l Jun 21, 2024

gazarenkov Jul 8, 2024

gazarenkov Jul 8, 2024

github-actions bot commented Jul 8, 2024

github-actions bot commented Jul 8, 2024

github-actions bot commented Jul 8, 2024

openshift-merge-robot commented Jul 12, 2024

Deployment configuration and v1alpha2 version #388

Are you sure you want to change the base?

Deployment configuration and v1alpha2 version #388

Conversation

gazarenkov commented Jun 20, 2024 • edited Loading

Description

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

PR acceptance criteria

How to test changes / Special notes to the reviewer

openshift-ci bot commented Jun 20, 2024

github-actions bot commented Jun 20, 2024

github-actions bot commented Jun 20, 2024

rm3l Jun 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jul 8, 2024

github-actions bot commented Jul 8, 2024

github-actions bot commented Jul 8, 2024

openshift-merge-robot commented Jul 12, 2024

gazarenkov commented Jun 20, 2024 •

edited

Loading

rm3l Jun 21, 2024 •

edited

Loading