volume: Add support for volume size and env variable overrides (PROJQUAY-1090) #586

jonathankingfc · 2021-11-04T19:23:28Z

Add overrides to component spec
Add volumeSize to overrides spec field

github-actions

See the ticket for this pull request: https://issues.redhat.com/browse/PROJQUAY-1090

dmesser · 2021-11-04T23:50:28Z

I propose to add arbitrary labels as well. This will help with a lot of use cases where customers want to leverage labels to base automation and customization upon. One particular example is router sharding in OpenShift (https://docs.openshift.com/container-platform/4.9/networking/ingress-operator.html#nw-ingress-sharding-route-labels_configuring-ingress)

apis/quay/v1/quayregistry_types.go

dmage · 2021-11-15T13:22:35Z

apis/quay/v1/quayregistry_types.go

-	ComponentMirror                      = "mirror"
-	ComponentMonitoring                  = "monitoring"
-	ComponentTLS                         = "tls"
+	ComponentBase          = "base"


I would keep these constants typed.

The way it was only the ComponentBase had a different type, all the others were strings: https://goplay.space/#C-DHpg_1hcO

Changed back to Componentkind type

I think you gonna have to add ComponentKind to each line, otherwise only the first one is of type ComponentKind. Can you do that?

ricardomaraschini

Please take a look at my comments.

ricardomaraschini · 2021-11-29T17:16:17Z

apis/quay/v1/quayregistry_types.go

-	ComponentMirror                      = "mirror"
-	ComponentMonitoring                  = "monitoring"
-	ComponentTLS                         = "tls"
+	ComponentBase          = "base"


The way it was only the ComponentBase had a different type, all the others were strings: https://goplay.space/#C-DHpg_1hcO

apis/quay/v1/quayregistry_types.go

pkg/middleware/middleware.go

apis/quay/v1/quayregistry_types.go

pkg/middleware/middleware.go

flavianmissi

Hey @jonathankingfc,
I thought we had agreed that we would focus on overrides, but this PR includes both labels and env vars.
What are you plans here?

jonathankingfc · 2021-11-30T14:28:02Z

Hey @jonathankingfc, I thought we had agreed that we would focus on overrides, but this PR includes both labels and env vars. What are you plans here?

@flavianmissi Updated the PR with fixes proposed by @ricardomaraschini and removed any implementation of env and label overrides (this should be supported in a later PR).

apis/quay/v1/quayregistry_types.go

pkg/middleware/middleware.go

ricardomaraschini

Almost there, just clarifying some things below.

ricardomaraschini · 2021-12-02T18:18:13Z

apis/quay/v1/quayregistry_types.go

+var supportsVolumeOverride = []ComponentKind{
+	ComponentPostgres,
+	ComponentClair,
+	ComponentObjectStorage,


Does ObjectStorage support Override ? The Jira card has, as an acceptance criteria:

"volume overrides available for every managed component that uses a PersistentVolume (Noobaa Backend, Postgres for Quay, Postgres for Clair)"

I know that we do not set the size of the Nooba Backend but before merging this we better clarify the behavior with the Jira card reporter.

pkg/middleware/middleware.go

apis/quay/v1/quayregistry_types.go

pkg/middleware/middleware.go

ricardomaraschini

All good! Let's just await for @dmesser in the Jira card then we can get this in.

ricardomaraschini · 2021-12-03T18:20:02Z

pkg/middleware/middleware.go

+		if volumeSizeOverride != nil {
+			// Ensure that volume size is not being reduced
+			if pvc.Spec.Resources.Requests.Storage() != nil && volumeSizeOverride.Cmp(*pvc.Spec.Resources.Requests.Storage()) == -1 {
+				return nil, fmt.Errorf("cannot shrink volume override size from %s to %s", volumeSizeOverride.String(), pvc.Spec.Resources.Requests.Storage().String())


Aren't from and to inverted here?

@jonathankingfc this is the last one, I promise :-)

- Add override field to Operator CRD to allow for volume size overrides - Add tests to reflect changes in API

github-actions bot reviewed Nov 4, 2021

View reviewed changes

flavianmissi reviewed Nov 5, 2021

View reviewed changes

apis/quay/v1/quayregistry_types.go Outdated Show resolved Hide resolved

jonathankingfc force-pushed the PROJQUAY-1090 branch from cd77cc4 to 6861556 Compare November 10, 2021 13:51

dmage reviewed Nov 15, 2021

View reviewed changes

ricardomaraschini suggested changes Nov 29, 2021

View reviewed changes

ricardomaraschini reviewed Nov 29, 2021

View reviewed changes

apis/quay/v1/quayregistry_types.go Outdated Show resolved Hide resolved

ricardomaraschini reviewed Nov 29, 2021

View reviewed changes

pkg/middleware/middleware.go Outdated Show resolved Hide resolved

flavianmissi reviewed Nov 30, 2021

View reviewed changes

jonathankingfc force-pushed the PROJQUAY-1090 branch 2 times, most recently from d69afd2 to 9ec9cd3 Compare November 30, 2021 14:25

jonathankingfc force-pushed the PROJQUAY-1090 branch from 9ec9cd3 to f8eeea2 Compare November 30, 2021 14:50

jonathankingfc changed the title ~~[WIP] volume: Add support for volume size and env variable overrides (PROJQUAY-1090)~~ volume: Add support for volume size and env variable overrides (PROJQUAY-1090) Nov 30, 2021

jonathankingfc force-pushed the PROJQUAY-1090 branch from f8eeea2 to 9ed6371 Compare November 30, 2021 16:25

ricardomaraschini reviewed Nov 30, 2021

View reviewed changes

apis/quay/v1/quayregistry_types.go Outdated Show resolved Hide resolved

pkg/middleware/middleware.go Outdated Show resolved Hide resolved

jonathankingfc force-pushed the PROJQUAY-1090 branch 3 times, most recently from 0e1af36 to 573a3d0 Compare December 2, 2021 14:30

flavianmissi requested a review from ricardomaraschini December 2, 2021 14:44

ricardomaraschini suggested changes Dec 2, 2021

View reviewed changes

jonathankingfc force-pushed the PROJQUAY-1090 branch 2 times, most recently from 5a1ba49 to e93e2c3 Compare December 3, 2021 16:14

ricardomaraschini reviewed Dec 3, 2021

View reviewed changes

overrides: Allow for volume size overrides (PROJQUAY-1090)

8c0cce2

- Add override field to Operator CRD to allow for volume size overrides - Add tests to reflect changes in API

jonathankingfc force-pushed the PROJQUAY-1090 branch from e93e2c3 to 8c0cce2 Compare December 3, 2021 18:42

ricardomaraschini approved these changes Dec 3, 2021

View reviewed changes

jonathankingfc merged commit a1a1550 into master Dec 3, 2021

jonathankingfc added a commit that referenced this pull request Dec 3, 2021

overrides: Allow for volume size overrides (PROJQUAY-1090) (#586)

a520bb0

- Add override field to Operator CRD to allow for volume size overrides - Add tests to reflect changes in API

jonathankingfc added a commit that referenced this pull request Dec 3, 2021

overrides: Allow for volume size overrides (PROJQUAY-1090) (#586) (#592)

e89eb54

- Add override field to Operator CRD to allow for volume size overrides - Add tests to reflect changes in API

ricardomaraschini deleted the PROJQUAY-1090 branch December 15, 2021 12:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

volume: Add support for volume size and env variable overrides (PROJQUAY-1090) #586

volume: Add support for volume size and env variable overrides (PROJQUAY-1090) #586

jonathankingfc commented Nov 4, 2021 •

edited by ricardomaraschini

Loading

github-actions bot left a comment

dmesser commented Nov 4, 2021

dmage Nov 15, 2021

ricardomaraschini Nov 29, 2021

jonathankingfc Nov 30, 2021

ricardomaraschini Nov 30, 2021

ricardomaraschini left a comment

ricardomaraschini Nov 29, 2021

flavianmissi left a comment

jonathankingfc commented Nov 30, 2021 •

edited

Loading

ricardomaraschini left a comment

ricardomaraschini Dec 2, 2021

ricardomaraschini left a comment

ricardomaraschini Dec 3, 2021

ricardomaraschini Dec 3, 2021

jonathankingfc Dec 3, 2021

volume: Add support for volume size and env variable overrides (PROJQUAY-1090) #586

volume: Add support for volume size and env variable overrides (PROJQUAY-1090) #586

Conversation

jonathankingfc commented Nov 4, 2021 • edited by ricardomaraschini Loading

github-actions bot left a comment

Choose a reason for hiding this comment

dmesser commented Nov 4, 2021

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

ricardomaraschini left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

flavianmissi left a comment

Choose a reason for hiding this comment

jonathankingfc commented Nov 30, 2021 • edited Loading

ricardomaraschini left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ricardomaraschini left a 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

jonathankingfc commented Nov 4, 2021 •

edited by ricardomaraschini

Loading

jonathankingfc commented Nov 30, 2021 •

edited

Loading