Layered operator #272

gazarenkov · 2024-03-20T20:08:39Z

Description

Operator was redesigned in order to:

Have more testable and documentable layered design
Be closer to "desired state" driven operator design
Move majority of tests to test unit level and reduce number of controller tests (still have all the original controller tests in place to be able to compare)
Make it fit design document

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

#69

PR acceptance criteria

Tests
Documentation

How to test changes / Special notes to the reviewer

make test

# Conflicts: # .gitignore # controllers/backstage_controller.go # controllers/backstage_controller_test.go # controllers/backstage_deployment.go # controllers/backstage_service.go # controllers/local_db_deployment.go # controllers/local_db_storage.go

…tus2

# Conflicts: # api/v1alpha1/backstage_types.go # config/crd/bases/janus-idp.io_backstages.yaml # controllers/backstage_backend_auth.go # controllers/backstage_controller.go # controllers/backstage_controller_test.go # controllers/local_db_statefulset.go

# Conflicts: # go.mod

Co-authored-by: gazarenkov <gazarenkov@users.noreply.github.com>

github-actions · 2024-04-01T18:15:09Z

⚠️ 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-04-01T18:20:14Z

PR images are available:

Makefile

examples/rhdh-cr-with-app-configs.yaml

rm3l · 2024-04-03T09:26:22Z

api/v1alpha1/backstage_types.go

@@ -98,7 +111,7 @@ type Application struct {

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


If ImagePullSecrets []string is not defined it contains nil pointer If ImagePullSecrets []string is initialized with empty array it contains pointer to empty slice with len:0 and cap:0

So, yes we can distinguish just checking ImagePullSecrets == nil Is it what you're asking about?

It is just that, as it is, the way it is iterated upon in this PR (https://github.com/gazarenkov/janus-idp-operator/blob/e6c56cecebf4f76748e8c80903059593e7fd78a8/pkg/model/deployment.go#L188-L191) will never allow CR users to explicitly set this list to an empty list.

This used to work previously (see

operator/controllers/backstage_deployment.go

Lines 219 to 224 in 496da58

if backstage.Spec.Application.ImagePullSecrets != nil { // use image pull secrets from the CR spec

deployment.Spec.Template.Spec.ImagePullSecrets = nil

if len(*backstage.Spec.Application.ImagePullSecrets) > 0 {

for _, imagePullSecret := range *backstage.Spec.Application.ImagePullSecrets {

deployment.Spec.Template.Spec.ImagePullSecrets = append(deployment.Spec.Template.Spec.ImagePullSecrets, v1.LocalObjectReference{

Name: imagePullSecret,

), i.e., if I explicitly set imagePullSecrets: [] in my CR, the resulting Deployment won't have any pull secrets defined, and if I don't set imagePullSecrets in the CR, it will inherit those coming from the operator/raw config.

But this can be tackled in a separate issue.

Co-authored-by: Armel Soro <armel@rm3l.org>

github-actions · 2024-04-03T15:37:03Z

PR images are available:

Makefile

sonarcloud · 2024-04-03T16:00:25Z

Quality Gate passed

Issues
11 New issues
6 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

github-actions · 2024-04-03T16:05:26Z

PR images are available:

rm3l

Approving to unblock other work items, but besides what's already in the review comments, there are a couple of things that have an impact on the current downstream documentation. We need to make sure those are handled later in the product documentation:

the way the backend auth secret is handled
to see changes from a custom CM already referenced in CR, either doing oc rollout restart of the Deployment or updating the spec.application.replicas field in the CR to 0 then back to what it was, so that the operator can recreate the Deployment with the new changes
changes to the default naming pattern of the route URL: changed from https://backstage-<CUSTOM_RESOURCE_NAME>-<NAMESPACE_NAME>.<OPENSHIFT_INGRESS_DOMAIN> to https://<CUSTOM_RESOURCE_NAME>-backstage-<NAMESPACE_NAME>.<OPENSHIFT_INGRESS_DOMAIN>

openshift-ci · 2024-04-03T16:10:34Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rm3l

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [rm3l]

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

gazarenkov added 30 commits November 28, 2023 11:25

yaml/configMap default configuration

cfb83b2

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

c952470

fix make test

9563358

fix with new objects

99b4e54

fix with new objects

cffa417

config small fixes

2bf0716

fix for janus-idp#51

47ba2f9

Merge branch 'main' of https://github.com/janus-idp/operator into sta…

5882f62

…tus2

Merge branch 'main' of https://github.com/janus-idp/operator into doc

4863874

fix for janus-idp#58

4bdb1a0

init next (design improvement)

971a50c

initial model

93d3de7

initial model

6da447e

initial

8d8b84c

initial

ac60557

initial

eb5f305

format and license

19d923d

factory and pswd generator

267a166

delete onCreate handler

20fbc03

support configmapfiles, dynamic-plugins

089f56c

initial model test framework

a1a671a

configurations

a138de1

more comments and tests

48a88ab

add more tests, remove old logic of object creation

8436faa

add more tests, remove old logic of object creation

18119c6

add support of keys, integration tests passed

7f937ba

add support of keys, integration tests passed

bb311a7

fix npe

63254a0

gazarenkov added 4 commits April 1, 2024 21:05

small fixes

e84d054

small fixes

ee299e8

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

fd77255

# Conflicts: # go.mod

merge

da2b990

gazarenkov temporarily deployed to internal April 1, 2024 18:14 — with GitHub Actions Inactive

openshift-merge-robot removed the needs-rebase label Apr 1, 2024

Regenerate bundle manifests

e6c56ce

Co-authored-by: gazarenkov <gazarenkov@users.noreply.github.com>

gazarenkov mentioned this pull request Apr 1, 2024

Fix E2E tests by changing the list of extra dynamic plugins added to examples/rhdh-cr-with-app-configs.yaml #286

Closed

3 tasks

rm3l reviewed Apr 3, 2024

View reviewed changes

Update examples/rhdh-cr-with-app-configs.yaml

ab6b1bd

Co-authored-by: Armel Soro <armel@rm3l.org>

gazarenkov temporarily deployed to internal April 3, 2024 15:29 — with GitHub Actions Inactive

Update Makefile

9f9829d

Co-authored-by: Armel Soro <armel@rm3l.org>

gazarenkov temporarily deployed to internal April 3, 2024 15:30 — with GitHub Actions Inactive

gazarenkov temporarily deployed to internal April 3, 2024 15:31 — with GitHub Actions Inactive

rm3l reviewed Apr 3, 2024

View reviewed changes

Makefile Outdated Show resolved Hide resolved

Update Makefile

73f77d0

rm3l temporarily deployed to internal April 3, 2024 16:00 — with GitHub Actions Inactive

rm3l approved these changes Apr 3, 2024

View reviewed changes

openshift-ci bot added the lgtm PR is ready to be merged. Required by Prow. label Apr 3, 2024

openshift-ci bot added the approved PR has been approved by an approver from all required OWNERS files. Required by Prow. label Apr 3, 2024

openshift-merge-bot bot merged commit 0a8f41f into janus-idp:main Apr 3, 2024
9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Layered operator #272

Layered operator #272

gazarenkov commented Mar 20, 2024

github-actions bot commented Apr 1, 2024

github-actions bot commented Apr 1, 2024

rm3l Apr 3, 2024

github-actions bot commented Apr 3, 2024

sonarcloud bot commented Apr 3, 2024

github-actions bot commented Apr 3, 2024

rm3l left a comment

openshift-ci bot commented Apr 3, 2024

	if backstage.Spec.Application.ImagePullSecrets != nil { // use image pull secrets from the CR spec
	deployment.Spec.Template.Spec.ImagePullSecrets = nil
	if len(*backstage.Spec.Application.ImagePullSecrets) > 0 {
	for _, imagePullSecret := range *backstage.Spec.Application.ImagePullSecrets {
	deployment.Spec.Template.Spec.ImagePullSecrets = append(deployment.Spec.Template.Spec.ImagePullSecrets, v1.LocalObjectReference{
	Name: imagePullSecret,

Layered operator #272

Layered operator #272

Conversation

gazarenkov commented Mar 20, 2024

Description

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

PR acceptance criteria

How to test changes / Special notes to the reviewer

github-actions bot commented Apr 1, 2024

github-actions bot commented Apr 1, 2024

rm3l Apr 3, 2024

Choose a reason for hiding this comment

github-actions bot commented Apr 3, 2024

sonarcloud bot commented Apr 3, 2024

Quality Gate passed

github-actions bot commented Apr 3, 2024

rm3l left a comment

Choose a reason for hiding this comment

openshift-ci bot commented Apr 3, 2024