feat: update prometheus-operator dependency in go.mod #140

jan--f · 2022-05-18T07:10:48Z

This updates the prometheus-operator go dependency to v0.55.0. This
allows specifiying additional auth options in remote_write sections.

Signed-off-by: Jan Fajerski [email protected]

JoaoBraveCoding · 2022-05-18T07:51:18Z

Since we are updating the PO, do you think we could also take the opportunity to update the PO deployment under deploy/dependencies/prometheus-operator-deployment.yaml? Otherwise we would be running PO CRD from v0.55.0 and a deployment from v0.50.0.

jan--f · 2022-05-18T08:03:23Z

Since we are updating the PO, do you think we could also take the opportunity to update the PO deployment under deploy/dependencies/prometheus-operator-deployment.yaml? Otherwise we would be running PO CRD from v0.55.0 and a deployment from v0.50.0.

Good point yes. The deployment was weird anyway, it claimed to be v0.52.1 and used the config reloader from that tag. But a 0.50.0 image was deployed... 🤔
We should probably automate something here?

JoaoBraveCoding

We should probably automate something here?

100% agree.

LGTM 👍 just two last comments:

we might want to ship v0.55.1 since it contains a bugfix for queryLogFile.
to make everything consistent we might also want to update the PO jsonnet version, under jsonnet/jsonnetfile.json, it's still pointing to v0.52.1. AFAIK this is only being used to generate PrometheusRules and it will not change anything but it will make things consistent.

simonpasquier

lgtm

sthaha

Requesting Changes so that this does not get merged by mistake.

Like @JoaoBraveCoding pointed out, I think we should update all the files in deploy/dependencies including cluster-roles, rolebindings etc to match what's upstream.

Next we need to update the deploy/crds/kubernetes so that tests on kind cluster will run fine.

Does updating to 0.55.1 also limit the openshift versions we support ?

jan--f · 2022-05-18T15:21:01Z

Ok lets list explicitly list what locations need updates and where/how they are to be updated. Feel free to edit this comment as needed.

Update go.mod in order to include types in prometheus-operator pkg/apis/monitoring/v1/types.go and run go mod tidy
Update po version in jsonnet/jsonnetfile.json
Update version tags in deploy/dependencies/*
Update deploy/crds/kubernetes with files from https://github.com/prometheus-operator/prometheus-operator/tree/<version>/example/prometheus-operator-crd/

sthaha · 2022-05-19T00:06:50Z

@jan--f I think you have list everything (including more than what I could think of).

I was wondering if we could replace the step (need not be in this PR but something worth thinking about) with generate using controller-gen the crds ? (~~I haven't tried this~~)

Update deploy/crds/kubernetes with files from https://github.com/prometheus-operator/prometheus-operator/tree//example/prometheus-operator-crd/

This seems to work fine.

.PHONY: generate-prom-operator-crds
generate-prom-operator-crds: $(CONTROLLER_GEN)
	$(CONTROLLER_GEN) crd \
		paths=github.com/prometheus-operator/prometheus-operator/pkg/apis/... \
		output:dir=. \
		output:crd:dir=./deploy/crds/kubernetes

But generation of rbac will need to be copied from upstream since I didn't find controllers using kubebuilder:rbac

Also

Update version tags in deploy/dependencies/*

Can we use some kustomize magic to do the replacement ?

jan--f · 2022-05-23T17:02:30Z

@jan--f I think you have list everything (including more than what I could think of).

I was wondering if we could replace the step (need not be in this PR but something worth thinking about) with generate using controller-gen the crds ? (~~I haven't tried this~~)

Update deploy/crds/kubernetes with files from https://github.com/prometheus-operator/prometheus-operator/tree//example/prometheus-operator-crd/

This seems to work fine.
.PHONY: generate-prom-operator-crds
generate-prom-operator-crds: $(CONTROLLER_GEN)
	$(CONTROLLER_GEN) crd \
		paths=github.com/prometheus-operator/prometheus-operator/pkg/apis/... \
		output:dir=. \
		output:crd:dir=./deploy/crds/kubernetes
But generation of rbac will need to be copied from upstream since I didn't find controllers using kubebuilder:rbac

Which rbac resource are you referring to? I think the only ones we can't generate should be covered by kustomize below?

As for the CRD generation, the above snippet works indeed. However it seems we don't have any control over the version that controller-gen uses with that path, right?
How about we simply shallow-clone the p-o repo in a temp dir and generate from that? That allows us to specify the commit we want and might make debugging a bit simpler if we ever need it.

Also

Update version tags in deploy/dependencies/*

Can we use some kustomize magic to do the replacement ?

Looks like it, check out d8d3d7f. I'd even be inclined to just generate those resources from the upstream examples. Wdyt?

This updates the prometheus-operator go dependency to v0.55.0. This allows specifiying additional auth options in remote_write sections. Signed-off-by: Jan Fajerski <[email protected]>

Problem: Embedded structs can as of yet not be directly referenced in struct literals. Solution: Explicitly use the embedded structs. Issues: golang/go#9859, prometheus-operator/prometheus-operator#4539 Signed-off-by: Jan Fajerski <[email protected]>

There is no need to track the prometheus-operator dependencies, they can just be generated from the upstream artifacts. Signed-off-by: Jan Fajerski <[email protected]>

Signed-off-by: Jan Fajerski <[email protected]>

jan--f · 2022-05-23T18:58:50Z

As for the CRD generation, the above snippet works indeed. However it seems we don't have any control over the version that controller-gen uses with that path, right?

Or does controller-gen simply respect what we have in go.mod? I fail to find any documentation about this 🤔

sthaha · 2022-05-24T03:39:20Z

Or does controller-gen simply respect what we have in go.mod? I fail to find any documentation about this 🤔

The initial implementation I had was to use go mod vendor and use the vendor/github.com/.. to generate. That way we are explicit about the version. I think it is worth reading the source a bit to see how it picks up the package to parse.

--- update ---

I tested with multiple version of prometheus-operator in go.mod and it it turns out controller-gen does indeed honour
go.mod. In fact it complains if you only update go.mod and does not find the modules locally (no go mod download .. )

❯ go clean -modcache
❯ make generate-prom-operator-crds

### on main branch gives us 
❯ ls ~/go/pkg/mod/github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring* -d
~go/pkg/mod/github.com/prometheus-operator/prometheus-operator/pkg/apis/[email protected]

### update to 0.55.1 in go.mod

❯ make generate-prom-operator-crds
 ### complains about module not downloaded 
❯ go mod download github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring   


❯ ls ~/go/pkg/mod/github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring* -d
~/go/pkg/mod/github.com/prometheus-operator/prometheus-operator/pkg/apis/[email protected]
~/go/pkg/mod/github.com/prometheus-operator/prometheus-operator/pkg/apis/[email protected]

So the new module got downloaded 

### clean module and rerun it 
❯ go clean -modcache

❯ make generate-prom-operator-crds

❯ ls ~/go/pkg/mod/github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring* -d
~/go/pkg/mod/github.com/prometheus-operator/prometheus-operator/pkg/apis/[email protected]

The above gives me enough confidence to use controller-gen

deploy/dependencies/kustomization.yaml

jan--f · 2022-05-24T09:58:53Z

Awesome, I like the go.mod and controller-gen setup. Will add this to the PR.

JoaoBraveCoding · 2022-05-24T10:03:37Z

Awesome work! ❤️ Looking forward to how it looks in the end!

sthaha · 2022-05-30T05:54:46Z

Makefile

+		paths=github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/... \
+		output:dir=. \
+		output:crd:dir=./deploy/crds/kubernetes
+
 .PHONY: generate-crds
 generate-crds: $(CONTROLLER_GEN)


I feel generate-crds should generate prom crds as well, WDYT?

sthaha · 2022-05-30T05:55:20Z

deploy/crds/kubernetes/kustomization.yaml

@@ -1,11 +1,11 @@
---
+----


isn't it 3 - -> --- ? 🤔

This adds a Makefile target to generate the prometheus-operator related CRD manifests with controller-gen. For this controller-gen will respect the prometheus-operator version specified in go.mod. Signed-off-by: Jan Fajerski <[email protected]>

sthaha · 2022-05-30T06:09:13Z

Nice improvement @jan--f ❤️

jan--f requested a review from a team May 18, 2022 07:10

jan--f force-pushed the update-po-go-dep branch from 25fa589 to c3c15eb Compare May 18, 2022 07:47

JoaoBraveCoding reviewed May 18, 2022

View reviewed changes

jan--f mentioned this pull request May 18, 2022

chore(release): 0.0.8 #136

Merged

simonpasquier previously approved these changes May 18, 2022

View reviewed changes

sthaha requested changes May 18, 2022

View reviewed changes

jan--f dismissed simonpasquier’s stale review via 8532319 May 23, 2022 16:51

jan--f force-pushed the update-po-go-dep branch from f2348ca to 8532319 Compare May 23, 2022 16:51

jan--f added 4 commits May 23, 2022 20:41

feat: update prometheus-operator dependency in go.mod

ee1c6ea

This updates the prometheus-operator go dependency to v0.55.0. This allows specifiying additional auth options in remote_write sections. Signed-off-by: Jan Fajerski <[email protected]>

refactor: generate dependencies from prometheus-operator repo

85ba170

There is no need to track the prometheus-operator dependencies, they can just be generated from the upstream artifacts. Signed-off-by: Jan Fajerski <[email protected]>

feat: bump prometheus-operator dependency to v0.55.1

9b6e62a

Signed-off-by: Jan Fajerski <[email protected]>

jan--f force-pushed the update-po-go-dep branch from 8532319 to 9b6e62a Compare May 23, 2022 18:42

sthaha reviewed May 24, 2022

View reviewed changes

deploy/dependencies/kustomization.yaml Show resolved Hide resolved

tsisodia10 mentioned this pull request May 25, 2022

Expose Prometheus OAuth2 in MSO #144

Closed

sthaha reviewed May 30, 2022

View reviewed changes

jan--f force-pushed the update-po-go-dep branch from 34f5970 to 76ff6fd Compare May 30, 2022 05:57

jan--f force-pushed the update-po-go-dep branch from 76ff6fd to de31295 Compare May 30, 2022 06:04

sthaha approved these changes May 30, 2022

View reviewed changes

jan--f merged commit fd6b78c into rhobs:main May 30, 2022

jan--f mentioned this pull request Jun 1, 2022

MSO Integration with Observatorium #127

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: update prometheus-operator dependency in go.mod #140

feat: update prometheus-operator dependency in go.mod #140

jan--f commented May 18, 2022

JoaoBraveCoding commented May 18, 2022

jan--f commented May 18, 2022

JoaoBraveCoding left a comment

simonpasquier left a comment

sthaha left a comment

jan--f commented May 18, 2022 •

edited

Loading

sthaha commented May 19, 2022 •

edited

Loading

jan--f commented May 23, 2022

jan--f commented May 23, 2022

sthaha commented May 24, 2022 •

edited

Loading

jan--f commented May 24, 2022

JoaoBraveCoding commented May 24, 2022

sthaha May 30, 2022

sthaha May 30, 2022

sthaha commented May 30, 2022

feat: update prometheus-operator dependency in go.mod #140

feat: update prometheus-operator dependency in go.mod #140

Conversation

jan--f commented May 18, 2022

JoaoBraveCoding commented May 18, 2022

jan--f commented May 18, 2022

JoaoBraveCoding left a comment

Choose a reason for hiding this comment

simonpasquier left a comment

Choose a reason for hiding this comment

sthaha left a comment

Choose a reason for hiding this comment

jan--f commented May 18, 2022 • edited Loading

sthaha commented May 19, 2022 • edited Loading

jan--f commented May 23, 2022

jan--f commented May 23, 2022

sthaha commented May 24, 2022 • edited Loading

jan--f commented May 24, 2022

JoaoBraveCoding commented May 24, 2022

sthaha May 30, 2022

Choose a reason for hiding this comment

sthaha May 30, 2022

Choose a reason for hiding this comment

sthaha commented May 30, 2022

jan--f commented May 18, 2022 •

edited

Loading

sthaha commented May 19, 2022 •

edited

Loading

sthaha commented May 24, 2022 •

edited

Loading