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

chore(updatecli): retrieve nodejs version from packer-images #146

Conversation

lemeurherve
Copy link
Member

@lemeurherve lemeurherve commented Jun 17, 2024

This PR adds an updatecli manifest to retrieve nodejs_linux_version from https://github.com/jenkins-infra/packer-images/blob/main/provisioning/tools-versions.yml so it's the same as the one installed on agents.

Closes:

Closes #127 (comment) (to validate updatecli main build)

@lemeurherve lemeurherve added the enhancement New feature or request label Jun 17, 2024
@lemeurherve lemeurherve requested a review from a team as a code owner June 17, 2024 08:26
@lemeurherve lemeurherve changed the title feat: retrieve nodejs version from packer-images feat(updatecli): retrieve nodejs version from packer-images Jun 17, 2024
Copy link
Contributor

@dduportal dduportal left a comment

Choose a reason for hiding this comment

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

As per jenkins-infra/kubernetes-management#5312 (review), you might want to add a Jenkinsfile_updatecli as part of this PR to specify the execution to infra.ci, WDYT?

Copy link
Contributor

@dduportal dduportal left a comment

Choose a reason for hiding this comment

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

The following elements are LGTM:

  • Pipeline works on infra.ci and the updatecli pipeline works as expected
  • The pipeline currently update NodeJS with success (as per the updatecli diff output)

👏

However, I got a blocking elements (and 2 nitpicks, e.g. non blocking the PR to be merged): you are using the branch main of packer-images as source of truth. But it is not. The source of truth is the version of packer image deployed in production (see my line comment)

updatecli/updatecli.d/nodejs.yaml Outdated Show resolved Hide resolved
updatecli/updatecli.d/nodejs.yaml Outdated Show resolved Hide resolved
@dduportal
Copy link
Contributor

Another nit: PR title is a chore, not a feature

@lemeurherve lemeurherve changed the title feat(updatecli): retrieve nodejs version from packer-images chore(updatecli): retrieve nodejs version from packer-images Jun 19, 2024
@lemeurherve lemeurherve marked this pull request as draft June 19, 2024 22:35
Copy link
Contributor

@dduportal dduportal left a comment

Choose a reason for hiding this comment

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

11:23:53  --- .tool-versions
11:23:53  +++ .tool-versions
11:23:53  @@ -1 +1 @@
11:23:53  -nodejs 20.12.2
11:23:53  +nodejs 20.14.0

=> from the check for updatecli diff

LGTM 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request updatecli
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants