Migrate to kubebuilder v4 layout #1092

rubenvp8510 · 2024-11-18T01:29:40Z

No description provided.

codecov-commenter · 2024-11-18T14:03:58Z

Codecov Report

Attention: Patch coverage is 20.83333% with 114 lines in your changes missing coverage. Please review.

Project coverage is 58.64%. Comparing base (a29a921) to head (f6909e8).

Files with missing lines	Patch %	Lines
api/config/v1alpha1/zz_generated.deepcopy.go	0.00%	65 Missing ⚠️
cmd/root/options.go	35.71%	38 Missing and 7 partials ⚠️
api/config/v1alpha1/projectconfig_types.go	0.00%	2 Missing ⚠️
cmd/main.go	0.00%	1 Missing ⚠️
cmd/start/main.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1092      +/-   ##
==========================================
- Coverage   67.93%   58.64%   -9.29%     
==========================================
  Files         110      113       +3     
  Lines        8731    10145    +1414     
==========================================
+ Hits         5931     5950      +19     
- Misses       2503     3892    +1389     
- Partials      297      303       +6

Flag	Coverage Δ
unittests	`58.64% <20.83%> (-9.29%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

cmd/root/root.go

iblancasa · 2024-11-20T10:55:23Z

cmd/root/root.go

+		return err
 	}


Can you keep the message here? I think it gives context and help debugging stuff

I think this is fine, as we don't want to mask errors.

cmd/root/options.go

cmd/root/config.go

iblancasa · 2024-11-20T11:12:10Z

Some nits. Thanks!

Signed-off-by: Ruben Vargas <[email protected]>

rubenvp8510 · 2024-12-03T03:08:12Z

@iblancasa I've already address some of your comments

@iblancasa @andreasgerstmayr Could you review this ? Thanks!

Signed-off-by: Ruben Vargas <[email protected]>

cmd/root/options.go

andreasgerstmayr · 2024-12-04T13:34:00Z

lgtm overall, great work! 💯
I left one comment regarding the pprof endpoints.

Signed-off-by: Ruben Vargas <[email protected]>

andreasgerstmayr · 2024-12-09T12:01:25Z

Last thing before merging, did you run the OpenShift e2e tests and confirm that they're still working?
Because there are some changes related to the metrics (.Metrics.BindAddress) in this PR, we should make sure that the operator and operand metrics still work, and iirc they are only tested in the OpenShift e2e test suite.

Signed-off-by: Ruben Vargas <[email protected]>

rubenvp8510 · 2024-12-12T06:45:05Z

Pass almost all OCP tests except some due must gather issues

-- FAIL: chainsaw (398.06s)
    --- PASS: chainsaw/monitoring (124.40s)
    --- PASS: chainsaw/monolithic-monitoring (76.25s)
    --- PASS: chainsaw/red-metrics (197.41s)
    --- FAIL: chainsaw/monolithic-route (60.15s)
    --- PASS: chainsaw/monolithic-single-tenant-auth (72.51s)
    --- PASS: chainsaw/tls-singletenant-monolithic (77.18s)
    --- PASS: chainsaw/monolithic-multitenancy-static (79.68s)
    --- PASS: chainsaw/monolithic-multitenancy-openshift (80.40s)
    --- PASS: chainsaw/tls-singletenant (125.31s)
    --- PASS: chainsaw/tempo-single-tenant-auth (132.51s)
    --- FAIL: chainsaw/route (139.00s)
    --- PASS: chainsaw/multitenancy (145.20s)
    --- PASS: chainsaw/tempostack-resources (178.05s)
    --- PASS: chainsaw/component-replicas (202.22s

I don't understand why this step fails

        Missing: *sha*/olm/operator-tempo-*-tempo-operator.yaml
    | 00:38:13 | monolithic-route | Run the must-gather and verify the contents | SCRIPT    | ERROR |

@IshwarKanse Could you help me testing this on your side? May be is something on my environment. Thanks

andreasgerstmayr · 2024-12-12T11:51:44Z

        Missing: *sha*/olm/operator-tempo-*-tempo-operator.yaml
    | 00:38:13 | monolithic-route | Run the must-gather and verify the contents | SCRIPT    | ERROR |
@IshwarKanse Could you help me testing this on your side? May be is something on my environment. Thanks

I think the namespace where the operator is installed must end in tempo-operator. Does it work with the instructions here: https://github.com/grafana/tempo-operator/blob/main/RELEASE.md#running-e2e-tests-on-openshift ?

IshwarKanse · 2024-12-12T15:49:29Z

@andreasgerstmayr Yeah, we use openshift-tempo-operator namespace in CI. https://github.com/openshift/release/blob/master/ci-operator/config/openshift/grafana-tempo-operator/openshift-grafana-tempo-operator-main__upstream-ocp-4.16-amd64-aws-ipi-shared-vpc-phz-sts.yaml#L62 https://github.com/openshift/release/blob/master/ci-operator/step-registry/distributed-tracing/tests/tempo/upstream/distributed-tracing-tests-tempo-upstream-commands.sh I'll check this next week once I'm back from leave.

pavolloffay · 2024-12-13T14:39:25Z

Makefile

@@ -488,7 +488,7 @@ docs/spec/%: bundle/community/manifests/% | gen-api-docs
 	$(GEN_API_DOCS) < $^ > $@

 docs/operator/config.yaml: gen-api-docs
-	$(GEN_API_DOCS) -pkg github.com/grafana/tempo-operator/apis/config/v1alpha1 -type ProjectConfig -format multiline > $@
+	$(GEN_API_DOCS) -pkg github.com/grafana/tempo-operator/api/config/v1alpha1 -type ProjectConfig -format multiline > $@


Can you explain this change? Maybe a link to documentation?

I think the apis was used for multi version projects.

and our PROJECT uses multigroup

Not anymore. is part of the layout changes

New layout:

The directory apis was renamed to api to follow the standard The controller(s) directory has been moved under a new directory called internal and renamed to singular as well controller The main.go previously scaffolded in the root directory has been moved under a new directory called cmd

pavolloffay · 2024-12-13T14:41:45Z

PROJECT

@@ -1,6 +1,6 @@
 domain: grafana.com
 layout:
- go.kubebuilder.io/v3
+- go.kubebuilder.io/v4


Can you point me to the docs for this file?

Sure, you can see the detailed steps here:

https://book.kubebuilder.io/migration/manually_migration_guide_gov3_to_gov4

Step: Migrate the layout version into the PROJECT file

IshwarKanse · 2024-12-18T09:30:09Z

        Missing: *sha*/olm/operator-tempo-*-tempo-operator.yaml
    | 00:38:13 | monolithic-route | Run the must-gather and verify the contents | SCRIPT    | ERROR |
@IshwarKanse Could you help me testing this on your side? May be is something on my environment. Thanks
I think the namespace where the operator is installed must end in tempo-operator. Does it work with the instructions here: https://github.com/grafana/tempo-operator/blob/main/RELEASE.md#running-e2e-tests-on-openshift ?

@andreasgerstmayr I installed the operator in openshift-tempo-operator project and ran the tests. The must-gather steps pass.

--- PASS: chainsaw (2294.18s)
    --- PASS: chainsaw/ossm-monolithic-otel (386.10s)
    --- PASS: chainsaw/ossm-tempostack (349.65s)
    --- PASS: chainsaw/ossm-tempostack-otel (374.55s)
    --- PASS: chainsaw/otel-tempo-serverless (412.65s)
    --- PASS: chainsaw/tempo-serverless (314.67s)
    --- PASS: chainsaw/monitoring (159.00s)
    --- PASS: chainsaw/monolithic-monitoring (101.48s)
    --- PASS: chainsaw/red-metrics (196.09s)
    --- PASS: chainsaw/operator-metrics (20.92s)
    --- PASS: chainsaw/reconcile (106.44s)
    --- PASS: chainsaw/monolithic-receivers-tls (114.84s)
    --- PASS: chainsaw/monolithic-pv (95.61s)
    --- PASS: chainsaw/monolithic-s3-tls (122.56s)
    --- PASS: chainsaw/tempostack-extraconfig (132.30s)
    --- PASS: chainsaw/receivers-tls (152.79s)
    --- PASS: chainsaw/receivers-mtls (153.01s)
    --- PASS: chainsaw/monolithic-extraconfig (54.30s)
    --- PASS: chainsaw/monolithic-memory (81.23s)
    --- PASS: chainsaw/monolithic-ingestion-mtls (74.47s)
    --- PASS: chainsaw/monolithic-route (76.84s)
    --- PASS: chainsaw/monolithic-single-tenant-auth (82.56s)
    --- PASS: chainsaw/generate (71.76s)
    --- PASS: chainsaw/gateway (43.53s)
    --- PASS: chainsaw/route (156.66s)
    --- PASS: chainsaw/monolithic-multitenancy-static (92.42s)
    --- PASS: chainsaw/multitenancy (162.88s)
    --- PASS: chainsaw/tempo-single-tenant-auth (143.93s)
    --- PASS: chainsaw/monolithic-multitenancy-openshift (80.69s)
    --- PASS: chainsaw/custom-ca (125.01s)
    --- PASS: chainsaw/tls-singletenant-monolithic (95.24s)
    --- PASS: chainsaw/compatibility (152.09s)
    --- PASS: chainsaw/tls-singletenant (157.65s)
    --- PASS: chainsaw/component-replicas (234.11s)
    --- PASS: chainsaw/tempostack-resources (197.49s)
    --- PASS: chainsaw/tempostack-retention-global (2862.40s)
PASS
Tests Summary...
- Passed  tests 35
- Failed  tests 0
- Skipped tests 0
Done.

rubenvp8510 force-pushed the migrate_kubebuilder_v4 branch 4 times, most recently from 56c0231 to 0a595ac Compare November 18, 2024 01:56

rubenvp8510 added the Skip Changelog label Nov 18, 2024

rubenvp8510 marked this pull request as ready for review November 18, 2024 06:09

rubenvp8510 requested review from andreasgerstmayr, pavolloffay, frzifus and iblancasa November 18, 2024 14:33

rubenvp8510 mentioned this pull request Nov 19, 2024

Update controller-runtime, controller-gen and other dependencies #1091

Closed

iblancasa reviewed Nov 20, 2024

View reviewed changes

rubenvp8510 requested a review from iblancasa December 3, 2024 03:04

rubenvp8510 added 7 commits December 2, 2024 21:05

Migrate to kubebuilder v4 layout

17a23c9

Signed-off-by: Ruben Vargas <[email protected]>

remove deprecated config component

1d17f7b

Signed-off-by: Ruben Vargas <[email protected]>

modifications to config

e9adbab

Signed-off-by: Ruben Vargas <[email protected]>

reintroduce config validation

d010417

Signed-off-by: Ruben Vargas <[email protected]>

bump sdk to 1.36

be9374c

Signed-off-by: Ruben Vargas <[email protected]>

update documents

fe1cfc7

Signed-off-by: Ruben Vargas <[email protected]>

address observations, merge files, add tests

02466ad

Signed-off-by: Ruben Vargas <[email protected]>

rubenvp8510 force-pushed the migrate_kubebuilder_v4 branch from 34d2a4a to 02466ad Compare December 3, 2024 03:07

fix lint

ab36568

Signed-off-by: Ruben Vargas <[email protected]>

iblancasa approved these changes Dec 3, 2024

View reviewed changes

andreasgerstmayr reviewed Dec 4, 2024

View reviewed changes

cmd/root/options.go Outdated Show resolved Hide resolved

rubenvp8510 force-pushed the migrate_kubebuilder_v4 branch from d9fe4e0 to ab36568 Compare December 9, 2024 05:10

remove pprof endpoints

2a60591

Signed-off-by: Ruben Vargas <[email protected]>

add namepace permissions

f6909e8

Signed-off-by: Ruben Vargas <[email protected]>

pavolloffay reviewed Dec 13, 2024

View reviewed changes

andreasgerstmayr approved these changes Dec 18, 2024

View reviewed changes

andreasgerstmayr mentioned this pull request Dec 18, 2024

[Chore] Use OTEL operator with pinned version in upstream CI and fix tests #1086

Open

rubenvp8510 merged commit c2c2f7b into grafana:main Dec 18, 2024
11 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate to kubebuilder v4 layout #1092

Migrate to kubebuilder v4 layout #1092

rubenvp8510 commented Nov 18, 2024

codecov-commenter commented Nov 18, 2024 •

edited

Loading

iblancasa Nov 20, 2024

rubenvp8510 Dec 3, 2024

iblancasa commented Nov 20, 2024

rubenvp8510 commented Dec 3, 2024

andreasgerstmayr commented Dec 4, 2024 •

edited

Loading

andreasgerstmayr commented Dec 9, 2024

rubenvp8510 commented Dec 12, 2024 •

edited

Loading

andreasgerstmayr commented Dec 12, 2024

IshwarKanse commented Dec 12, 2024

pavolloffay Dec 13, 2024

pavolloffay Dec 13, 2024

rubenvp8510 Dec 14, 2024

pavolloffay Dec 13, 2024

rubenvp8510 Dec 14, 2024

IshwarKanse commented Dec 18, 2024

Migrate to kubebuilder v4 layout #1092

Migrate to kubebuilder v4 layout #1092

Conversation

rubenvp8510 commented Nov 18, 2024

codecov-commenter commented Nov 18, 2024 • edited Loading

Codecov Report

iblancasa Nov 20, 2024

Choose a reason for hiding this comment

rubenvp8510 Dec 3, 2024

Choose a reason for hiding this comment

iblancasa commented Nov 20, 2024

rubenvp8510 commented Dec 3, 2024

andreasgerstmayr commented Dec 4, 2024 • edited Loading

andreasgerstmayr commented Dec 9, 2024

rubenvp8510 commented Dec 12, 2024 • edited Loading

andreasgerstmayr commented Dec 12, 2024

IshwarKanse commented Dec 12, 2024

pavolloffay Dec 13, 2024

Choose a reason for hiding this comment

pavolloffay Dec 13, 2024

Choose a reason for hiding this comment

rubenvp8510 Dec 14, 2024

Choose a reason for hiding this comment

pavolloffay Dec 13, 2024

Choose a reason for hiding this comment

rubenvp8510 Dec 14, 2024

Choose a reason for hiding this comment

IshwarKanse commented Dec 18, 2024

codecov-commenter commented Nov 18, 2024 •

edited

Loading

andreasgerstmayr commented Dec 4, 2024 •

edited

Loading

rubenvp8510 commented Dec 12, 2024 •

edited

Loading