THREESCALE-8535 OSD 4.12 support #785

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Merged

MStokluska merged 17 commits into 3scale:master from MStokluska:THREESCALE-8535

Oct 25, 2022

Contributor

MStokluska commented Oct 3, 2022 •

edited by eguzki

Loading

What's done so far

bumped Go to 1.17
bumped k8s deps to 0.24.3
bumped client-go to 0.24.3
bumped controller-runtime v0.12.2
bumped controller-gen 0.9.2

4.12 support

Replaced the v1beta1 policies with v1

Other changes to suit deps bumps

slight refactor to reconcilers
slight refactor to apimanager watches
update to single unit test
update to some deprecated logr commands
other changes supporting the dep bumps

Unfortunately, deps could not be bumped any further due to the fact that it would have clashed with other operators that RHOAM has to keep in line with.

TODO:
Some PR tests still failing as this would need to get merged to get them pass: openshift/release#33016
On cluster verification of initial install and upgrade


          THREESCALE-8535 initial bump

8ec3b1d

openshift-ci bot added the do-not-merge/work-in-progress label

MStokluska added 3 commits

October 4, 2022 09:56


          bump policy/v1beta to policy/v1

8cded48


          other updates to support 4.11

e9c59fd


          tidy up go mod

ca6ed6b

MStokluska force-pushed the THREESCALE-8535 branch from 890cc20 to ca6ed6b Compare

October 10, 2022 07:06

MStokluska added 5 commits

October 10, 2022 19:03


          update deps and code changes

493efff


          docker file and circleci changes

a5669dd


          fix crds

abb493c


          fix unit tests

d3eeb52


          prometheus rules updates

3a7c0f8

MStokluska changed the title ~~[WIP] THREESCALE-8535 initial bump~~ THREESCALE-8535 OSD 4.12 support

openshift-ci bot removed the do-not-merge/work-in-progress label

MStokluska requested a review from eguzki

October 11, 2022 08:24

Contributor Author

MStokluska commented Oct 11, 2022

@eguzki I would appreciate it if you could have a look at it, main areas I'm concerned with are:

apimanager reconciler watcher update
grafana bump to v4 and removal on the spec.name field (yet to do on cluster upgrade verification)


          removing unnecessary comments and adding comment to dep in go mod

389d30d

Contributor Author

MStokluska commented Oct 11, 2022

/test test-e2e
/test test-unit

eguzki requested changes

View reviewed changes

Member

eguzki left a comment

Awesome work 🎖️

Left some comments

This graphana major version bump needs to be analyzed. What happens when upgrading from earlier version and 3scale has grafana dashboards deployed? The grafana operator needs to be upgraded, before the 3scale operator I guess. Needs to be documented.

doc/prometheusrules/apicast.yaml

    
                      summary: Job {{ $labels.job }} on {{ $labels.namespace }} is DOWN

                    expr: up{job=~".*/apicast-production|.*/apicast-staging",namespace="__NAMESPACE__"} == 0

                    expr: up{job=~".*/apicast-production|.*/apicast-staging",namespace="__NAMESPACE__"}

                      == 0

Member

eguzki Oct 11, 2022

These changes have nothing to do with the purpose of the PR, have they?

If not, I would revert the changes

Contributor Author

MStokluska Oct 11, 2022

Hey, this was done in order to get the prometheus rules tests to pass. I am not sure as of yet why is this required to get the tests pass (what changes the expected format) but will dig deeper.

doc/prometheusrules/backend-listener.yaml

    
                  rules:

                  - alert: ThreescaleBackendListener5XXRequestsHigh

                    annotations:

                      description: Job {{ $labels.job }} on {{ $labels.namespace }} has more than 5000 HTTP 5xx requests in the last 5 minutes

Member

eguzki Oct 11, 2022

These changes have nothing to do with the purpose of the PR, have they?

If not, I would revert the changes

doc/prometheusrules/backend-worker.yaml

    
                  - alert: ThreescaleBackendWorkerJobsCountRunningHigh

                    annotations:

                      description: '{{$labels.container_name}} replica controller on {{$labels.namespace}} project: Has more than 1000 jobs processed in the last 5 minutes'

                      description: '{{$labels.container_name}} replica controller on {{$labels.namespace}}

Member

eguzki Oct 11, 2022

These changes have nothing to do with the purpose of the PR, have they?

If not, I would revert the changes

doc/prometheusrules/system-app.yaml

    
                  - alert: ThreescaleSystemApp5XXRequestsHigh

                    annotations:

                      description: Job {{ $labels.job }} on {{ $labels.namespace }} has more than 50 HTTP 5xx requests in the last minute

                      description: Job {{ $labels.job }} on {{ $labels.namespace }} has more than

Member

eguzki Oct 11, 2022

These changes have nothing to do with the purpose of the PR, have they?

If not, I would revert the changes

doc/prometheusrules/threescale-kube-state-metrics.yaml

    
                  - alert: ThreescalePodCrashLooping

                    annotations:

                      message: Pod {{ $labels.namespace }}/{{ $labels.pod }} ({{ $labels.container }}) is restarting {{ printf "%.2f" $value }} times / 5 minutes.

                      message: Pod {{ $labels.namespace }}/{{ $labels.pod }} ({{ $labels.container

Member

eguzki Oct 11, 2022

These changes have nothing to do with the purpose of the PR, have they?

If not, I would revert the changes

pkg/3scale/amp/operator/ampimages_reconciler_test.go Outdated Show resolved Hide resolved

pkg/handlers/apimanager_managed_routes_event_mapper_test.go Outdated Show resolved Hide resolved

pkg/reconcilers/base_reconciler_test.go Outdated Show resolved Hide resolved

pkg/reconcilers/base_reconciler_test.go Outdated Show resolved Hide resolved

pkg/reconcilers/base_reconciler_test.go Outdated Show resolved Hide resolved

Member

eguzki commented Oct 11, 2022 •

edited

Loading

update Makefile's go-get-tool method to use go install instead of go get. Maybe rename the method to go-bin-install ??


          address comments

89aae13

MStokluska force-pushed the THREESCALE-8535 branch from eb9529d to 89aae13 Compare

October 11, 2022 12:01

Contributor Author

MStokluska commented Oct 11, 2022

/test test-e2e

Contributor Author

MStokluska commented Oct 11, 2022

E2E fails due to it being tested on ocp 4.6, which does not have policy/v1 but policy/v1beta1. I suggest bumping ocp testing clusters to at least 4.8

Contributor Author

MStokluska commented Oct 11, 2022

/test test-e2e

Member

eguzki commented Oct 11, 2022 •

edited

Loading

I suggest to bump also kustomize and controller-gen,

kustomize v4.5.7
controller-gen v0.9.2

Aligned with the apicast operator #180


          bump controller gen and kustomize

MStokluska force-pushed the THREESCALE-8535 branch 2 times, most recently from eb4a81f to c967486 Compare

October 11, 2022 15:13


          update crds post controller gen bump

401aa19

MStokluska force-pushed the THREESCALE-8535 branch from c967486 to 401aa19 Compare

October 11, 2022 15:34


          THREESCALE-8535 further bump to deps and code updates

e55f572

MStokluska force-pushed the THREESCALE-8535 branch from 14e5659 to e55f572 Compare

October 21, 2022 12:15

Contributor Author

MStokluska commented Oct 24, 2022

Note: We need to document the upgrade path for this version.
This version of the 3scale operator will only work with Grafana v4, therefore in order for monitoring to be functional the Grafana Operator needs to be upgraded to v4.

MStokluska force-pushed the THREESCALE-8535 branch from 3aaf644 to 299e883 Compare

October 24, 2022 10:23


          THREESCALE-8535 update templates titles

0f216bf

MStokluska force-pushed the THREESCALE-8535 branch from 299e883 to 0f216bf Compare

October 24, 2022 10:30


          THREESCALE-8535 fix dynamic discovery of grafana dashboards

a9e6cdc

MStokluska force-pushed the THREESCALE-8535 branch from f1c6ab1 to a9e6cdc Compare

October 24, 2022 12:12

MStokluska requested a review from eguzki

October 24, 2022 12:44

eguzki previously approved these changes

View reviewed changes

eguzki dismissed their stale review

October 24, 2022 15:25

missing development doc

Member

eguzki commented Oct 24, 2022

LGTM, before merging, update /doc/development.md accordingly

eguzki mentioned this pull request

Upgrade tooling and dependencies (part 2) 3scale/apicast-operator#184

Merged

eguzki requested changes

View reviewed changes

Member

eguzki left a comment

update /doc/development.md accordingly


          THREESCALE-8535 update development doc

28148d2

qlty-cloud-legacy bot commented Oct 25, 2022

Code Climate has analyzed commit 28148d2 and detected 32 issues on this pull request.

Here's the issue category breakdown:

Category	Count
Duplication	7
Style	25

View more on Code Climate.

MStokluska requested a review from eguzki

October 25, 2022 09:42

eguzki approved these changes

View reviewed changes

MStokluska merged commit 81d3abc into 3scale:master

This was referenced Oct 25, 2022

Threescale 7668 #778

Merged

RBAC permissions for the coordination/v1 API #789

Merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet