Skip to content
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.

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

operator: upgrade kubebuilder to v3 #626

Merged
merged 2 commits into from
Mar 9, 2023

Conversation

t0rr3sp3dr0
Copy link
Contributor

@t0rr3sp3dr0 t0rr3sp3dr0 commented Mar 7, 2023

Project was migrated to Kubebuilder v3 using their guidelines (https://go.kubebuilder.io/migration/legacy/v2vsv3.html).

Summary of changes performed:

  • moved operator/api/v1alpha1 to operator/apis/http/v1alpha1
  • moved operator/controllers to operator/controllers/http
  • upgraded dependencies
  • performed changes on operator/main.go and operator/controllers/http/httpscaledobject_controller.go to match new Operator SDK scaffolding
  • migrated tests from ginkgo v1 to v2
  • simplified K8s manifests
  • updated Makefiles, Dockerfiles, .gitignores, and .dockerignores
  • configured kubernetes/code-generator

Checklist

Fixes #625

@t0rr3sp3dr0 t0rr3sp3dr0 requested a review from a team as a code owner March 7, 2023 05:55
@t0rr3sp3dr0
Copy link
Contributor Author

t0rr3sp3dr0 commented Mar 7, 2023

I'll be fixing build and tests tomorrow

Copy link
Member

@JorTurFer JorTurFer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good! Nice improvement, some comments inline
Apart from the, should we add to the CI checks the code-gen verification?

@t0rr3sp3dr0 t0rr3sp3dr0 force-pushed the pedrotorres/issue-625 branch from 529b13a to 87161b6 Compare March 8, 2023 21:48
@JorTurFer
Copy link
Member

A rebase is needed @t0rr3sp3dr0

@t0rr3sp3dr0 t0rr3sp3dr0 force-pushed the pedrotorres/issue-625 branch from 87161b6 to a6b7a54 Compare March 8, 2023 21:58
@JorTurFer
Copy link
Member

JorTurFer commented Mar 8, 2023

Static Checks are failing. Errors like this File is not gci-ed with --skip-generated -s standard,default,prefix(github.com/kedacore/http-add-on) are raised because we split imports in 3 groups:

  • standard (golang runtime)
  • default (any dep from internet, including github.com/kedacore/keda)
  • internals (github.com/kedacore/http-add-on)
    I clarify this because the error log isn't 100% clear

There are other failures related with code-gen
Could you take a look? 🙏

@t0rr3sp3dr0 t0rr3sp3dr0 force-pushed the pedrotorres/issue-625 branch 2 times, most recently from 84348e0 to 166ba31 Compare March 8, 2023 22:56
@t0rr3sp3dr0
Copy link
Contributor Author

Thanks for the tip about the static checks! The error message is kind of cryptic.

@t0rr3sp3dr0 t0rr3sp3dr0 force-pushed the pedrotorres/issue-625 branch from 166ba31 to 5fd564b Compare March 8, 2023 23:02
@JorTurFer
Copy link
Member

e2e tests are failing because of this https://github.com/kedacore/http-add-on/actions/runs/4369312560/jobs/7642967702#step:8:88
I think that it's a change in kustomize

@t0rr3sp3dr0 t0rr3sp3dr0 force-pushed the pedrotorres/issue-625 branch 2 times, most recently from 7293f8c to 4aae719 Compare March 9, 2023 00:23
@t0rr3sp3dr0 t0rr3sp3dr0 force-pushed the pedrotorres/issue-625 branch 3 times, most recently from 99cdbdc to a88a552 Compare March 9, 2023 01:16
@t0rr3sp3dr0 t0rr3sp3dr0 force-pushed the pedrotorres/issue-625 branch from a88a552 to 279a145 Compare March 9, 2023 01:27
@t0rr3sp3dr0
Copy link
Contributor Author

Tests fixed!

Copy link
Member

@JorTurFer JorTurFer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AWESOME!
Thanks for the improvement! Only one small nit inline

@JorTurFer JorTurFer merged commit 1bfc7ab into kedacore:main Mar 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

migrate to kubebuilder v3
2 participants