-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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 CI and release go versions to 1.17.5 #11799
Conversation
@@ -15,7 +15,7 @@ references: | |||
images: | |||
# When updating the Go version, remember to also update the versions in the | |||
# workflows section for go-test-lib jobs. | |||
go: &GOLANG_IMAGE docker.mirror.hashicorp.services/circleci/golang:1.17.2 | |||
go: &GOLANG_IMAGE docker.mirror.hashicorp.services/cimg/go:1.17.5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New version of CircleCI images: link
Code changes LGTM but the build is failing:
|
Yeah looks like there's some diffs between their legacy images and new ones 😭 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
🍒 If backport labels were added before merging, cherry-picking will start automatically. To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/524347. |
🍒✅ Cherry pick of commit 8bdbe7c onto |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @kisunji for working on this. I left few questions.
One unrelated open question, I just noticed that we run integration tests against Nomad on version 0.8.x of Nomad. Is there a reason we don't run against a more recent version?
ember: &EMBER_IMAGE docker.mirror.hashicorp.services/circleci/node:14-browsers | ||
|
||
paths: | ||
test-results: &TEST_RESULTS_DIR /tmp/test-results | ||
|
||
cache: | ||
yarn: &YARN_CACHE_KEY consul-ui-v6-{{ checksum "ui/yarn.lock" }} | ||
rubygem: &RUBYGEM_CACHE_KEY static-site-gems-v1-{{ checksum "Gemfile.lock" }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this need to be removed ? Is something different about the docker image we are using?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The rubygem cache was being unused in the file so I just deleted it
- run: sudo apt-get update --allow-releaseinfo-change-suite --allow-releaseinfo-change-version && sudo apt-get install -y rsyslog | ||
- run: sudo service rsyslog start | ||
- run: *install-gotestsum |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we removing the gotestsum install step here ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kisunji mentioned this in the other PR, but this is already installed as part of cimg/go
We actually run tests on 0.8 nomad and |
I think 0.8 might have just been the latest release at the time we added this. I suspect we were supposed to update that to the latest release periodically, but we have not done that. Maybe we should only test |
IIRC 0.8 was a particular version we broke during a release and had to do an immediate point release to fix. |
No description provided.