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

Update go version to 1.22 #267

Merged
merged 1 commit into from
Nov 29, 2024

Conversation

upadhyeammit
Copy link
Contributor

PR Title 💥

Please title this PR with a summary of the change, along with the JIRA card number.

Suggested formats:

  1. Fixes/Refs #RHIROS-XXX - Title
  2. RHIROS-XXX Title
  3. RHINENG-XXX Title

Feel free to remove this section from PR description once done.

Why do we need this change? 💭

Please include the context of this change here. If the change depends on Kruize add details about that as well!

Documentation update? 📝

  • Yes
  • No

Security Checklist 🔒

Upon raising this PR please go through RedHatInsights/secure-coding-checklist

💂‍♂️ Checklist 🎯

  • Does this change depend on specific version of Kruize
    • If yes what is the version no:
    • Is that image available in production or needs deployment?
  • Bugfix
  • New Feature
  • Refactor
  • Unittests Added
  • DRY code
  • Dependency Added
  • DB Migration Added

Additional 📣

Feel free to add any other relevant details such as links, notes, screenshots, here.

@upadhyeammit
Copy link
Contributor Author

I am not linking the RHINENG-5181 to the pull request as the bugzilla attached to this ticket is not yet resolved with the Errata.
At the same time, if I see the go releases then only recent two releases are supported. That does mean 1.23 and 1.22 however the ubi8/go-toolset is only having 1.21.13 as the latest tag which was released 23 days back. Looking at this, mostly we need to find other way to compile to go code as it seems go-toolset is lagging a lot!

@kgaikwad kgaikwad requested review from patilsuraj767 and saltgen and removed request for patilsuraj767 October 30, 2024 09:34
@saltgen
Copy link
Contributor

saltgen commented Nov 6, 2024

@upadhyeammit Shall we look around for some other image? I would like to check if there is a possibility to go for 1.22 or 1.23 atm.
Additionally, looks like the PR check is failing.

@saltgen
Copy link
Contributor

saltgen commented Nov 11, 2024

/retest

@upadhyeammit upadhyeammit force-pushed the update-go-packages branch 5 times, most recently from 02fb92d to 354129b Compare November 14, 2024 08:57
@upadhyeammit upadhyeammit changed the title Update go version to 1.21 Update go version to 1.22 Nov 14, 2024
go.mod Outdated
@@ -1,78 +1,87 @@
module github.com/redhatinsights/ros-ocp-backend

go 1.20
go 1.22.0
Copy link
Contributor

Choose a reason for hiding this comment

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

@upadhyeammit I think it is better to avoid the minor version here, since we don't seem to have that granularity in the Dockerfile tag, thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah.. actually its kind of like if we run the go mod tidy then it adds the minor version, updated it

@saltgen
Copy link
Contributor

saltgen commented Nov 14, 2024

@upadhyeammit Let's update the version here as well.

@upadhyeammit
Copy link
Contributor Author

@upadhyeammit Let's update the version here as well.

added already

@upadhyeammit
Copy link
Contributor Author

Its weird, if we remove the minor version then linting fails,

$ make lint
/home/amit/work/insights/ros/ros-ocp-backend/bin/golangci-lint run --timeout=3m ./...
ERRO Running error: context loading failed: no go files to analyze: running `go mod tidy` may solve the problem 
make: *** [Makefile:79: lint] Error 5
amit@fedora ~/work/insights/ros/ros-ocp-backend (update-go-packages)$ git status
On branch update-go-packages
nothing to commit, working tree clean
amit@fedora ~/work/insights/ros/ros-ocp-backend (update-go-packages)$ go mod tidy
amit@fedora ~/work/insights/ros/ros-ocp-backend (update-go-packages)$ git status
On branch update-go-packages
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   go.mod

no changes added to commit (use "git add" and/or "git commit -a")
amit@fedora ~/work/insights/ros/ros-ocp-backend (update-go-packages)$ make lint
/home/amit/work/insights/ros/ros-ocp-backend/bin/golangci-lint run --timeout=3m ./...
amit@fedora ~/work/insights/ros/ros-ocp-backend (update-go-packages)$ git diff
diff --git a/go.mod b/go.mod
index c2959f6..a5fb282 100644
--- a/go.mod
+++ b/go.mod
@@ -1,6 +1,6 @@
 module github.com/redhatinsights/ros-ocp-backend
 
-go 1.22
+go 1.22.0

@upadhyeammit
Copy link
Contributor Author

Its weird, if we remove the minor version then linting fails,

$ make lint
/home/amit/work/insights/ros/ros-ocp-backend/bin/golangci-lint run --timeout=3m ./...
ERRO Running error: context loading failed: no go files to analyze: running `go mod tidy` may solve the problem 
make: *** [Makefile:79: lint] Error 5
amit@fedora ~/work/insights/ros/ros-ocp-backend (update-go-packages)$ git status
On branch update-go-packages
nothing to commit, working tree clean
amit@fedora ~/work/insights/ros/ros-ocp-backend (update-go-packages)$ go mod tidy
amit@fedora ~/work/insights/ros/ros-ocp-backend (update-go-packages)$ git status
On branch update-go-packages
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   go.mod

no changes added to commit (use "git add" and/or "git commit -a")
amit@fedora ~/work/insights/ros/ros-ocp-backend (update-go-packages)$ make lint
/home/amit/work/insights/ros/ros-ocp-backend/bin/golangci-lint run --timeout=3m ./...
amit@fedora ~/work/insights/ros/ros-ocp-backend (update-go-packages)$ git diff
diff --git a/go.mod b/go.mod
index c2959f6..a5fb282 100644
--- a/go.mod
+++ b/go.mod
@@ -1,6 +1,6 @@
 module github.com/redhatinsights/ros-ocp-backend
 
-go 1.22
+go 1.22.0

as its failing https://github.com/RedHatInsights/ros-ocp-backend/actions/runs/11835786586/job/32979299087?pr=267 . I am repushing with minor version

@upadhyeammit upadhyeammit force-pushed the update-go-packages branch 3 times, most recently from edfd317 to c3a160d Compare November 19, 2024 07:41
@saltgen
Copy link
Contributor

saltgen commented Nov 20, 2024

@upadhyeammit I see that the image has 1.22.7 so we could do something like this and go mod tidy does not modify this further.

go 1.22.7

toolchain go1.22.7

I also noticed, that the minor versions don't change in the images by looking the previous tags.
This also means we should probably have a look if the e2e runs as expected, locally.

@upadhyeammit
Copy link
Contributor Author

@upadhyeammit I see that the image has 1.22.7 so we could do something like this and go mod tidy does not modify this further.

go 1.22.7

toolchain go1.22.7

I also noticed, that the minor versions don't change in the images by looking the previous tags. This also means we should probably have a look if the e2e runs as expected, locally.

I tried it and now I can see go mod tidy just kept one entry,

$ git diff
diff --git a/go.mod b/go.mod
index 1f4cc93..95c1fbc 100644
--- a/go.mod
+++ b/go.mod
@@ -1,8 +1,6 @@
 module github.com/redhatinsights/ros-ocp-backend
 
-go 1.22.0
-
-toolchain go1.22.9
+go 1.22.7

@upadhyeammit
Copy link
Contributor Author

/retest

Copy link
Contributor

@saltgen saltgen left a comment

Choose a reason for hiding this comment

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

@upadhyeammit Thanks for the changes 👍🏼

@upadhyeammit upadhyeammit merged commit a3060b1 into RedHatInsights:main Nov 29, 2024
7 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants