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.

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

Upgrade go to 1.19 and related dependencies #7

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Upgrade go to 1.19 and related dependencies #7

wants to merge 2 commits into from

Conversation

ezio-auditore
Copy link
Collaborator

@ezio-auditore ezio-auditore commented Mar 6, 2023 •
edited
Loading

Upgrade list

  • controller-tools
  • operator-sdk
  • gingko
  • gomega
  • operator-framework
  • prometheus
  • client-go
  • controller-runtime
  • k8s api

Other changes

  • Promethues template
  • k8sservicemonitor template
  • golang ci lint version upgrade
  • removed deprecated ioutil package with os package
    The above is required since after the prometheus package upgrade the spec has changed

@nb-ohad
Merge pull request red-hat-storage#3 from bindrad/deployment_controller
4cd677c 
Changes required for ManagedFusionDeployment Controller
@ezio-auditore ezio-auditore requested review from bindrad and nb-ohad and removed request for bindrad March 6, 2023 13:13
@ezio-auditore ezio-auditore force-pushed the go_upgrade branch 2 times, most recently from 5096694 to 607965f Compare March 6, 2023 13:31
bindrad
bindrad requested changes Mar 6, 2023
View reviewed changes
.github/workflows/golangci-lint.yaml Outdated
@@ -18,5 +18,5 @@ jobs:
- name: golangci-lint
uses: golangci/golangci-lint-action@v3
with:
version: v1.46.2
version: v1.50.0
Copy link
Member

Choose a reason for hiding this comment

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

Why are we using v1.50.0?
latest version available is v1.51.2(latest)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

Makefile
ENVTEST_VERSION ?= latest
OPERATOR_SDK_VERSION ?= v1.21.0
OPERATOR_SDK_VERSION ?= v1.25.4
Copy link
Member

Choose a reason for hiding this comment

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

Any specific reason to use v1.25.4?
v1.27.0 is available

Copy link
Member

Choose a reason for hiding this comment

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

v1.25.0 and above supports multi-arch build support so why don't we add a target as suggested in version upgrade docs?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

v1.25.4 is recommended to be installed according to the docs
https://docs.okd.io/4.12/operators/operator_sdk/golang/osdk-golang-updating-projects.html

go.mod Outdated
k8s.io/utils v0.0.0-20220210201930-3a6ce19ff2f9 // indirect
sigs.k8s.io/structured-merge-diff/v4 v4.2.1 // indirect
k8s.io/utils v0.0.0-20230202215443-34013725500c // indirect
sigs.k8s.io/structured-merge-diff/v4 v4.2.3 // indirect
sigs.k8s.io/yaml v1.2.0 // indirect
)

Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we remove modules from replace section as we are upgrading our modules to a newer version?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

leelavg
leelavg approved these changes Mar 8, 2023
View reviewed changes
Copy link
Collaborator

@leelavg leelavg left a comment

Choose a reason for hiding this comment

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

  • with the changes happened to prom, what's the lowest prom-op and ocp that we require?

@ezio-auditore ezio-auditore marked this pull request as draft March 14, 2023 11:13
@openshift-merge-robot
Copy link

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

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

@bindrad bindrad bindrad requested changes

@leelavg leelavg leelavg approved these changes

@nb-ohad nb-ohad Awaiting requested review from nb-ohad

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
needs-rebase
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ezio-auditore @openshift-merge-robot @leelavg @bindrad @nb-ohad