Linkerd CLI Chocolatey Package by drholmie · Pull Request #4205 · linkerd/linkerd2

drholmie · 2020-03-27T14:31:31Z

This PR partially fixes #3063 by building a chocolatey package for Linkerd2's Windows CLI
It adds the build scripts for the Linkerd chocolatey package and based on discussions in
#3921

The package currently exists in the bin/win folder
The .nuspec script has been modified for easy integration with the workflow
done by @ihcsim here: https://github.com/ihcsim/linkerd-chocolatey

To build outside of workflow environment (on windows system):

Modify linkerd.nuspec version tag to an available stable version of Linkerd2's cli
eg: 2.7.0 and file tag's src to tools\**
Install chocolatey
cd into bin/win/
Run choco pack
Run choco install linkerd -s . (make sure cmd/ powershell is running as administrator)
Run refreshenv to add linkerd.exe path to system
To unistall run choco uninstall linkerd

Tackling requirements discussed in #3921
(specifically: #3921 (comment))

Followed folder structure as described here: https://github.com/ihcsim/linkerd-chocolatey
Modified package to accept version through the metadata available in .nuspec file
Modified package to override install path with command line arg/ env variable
To run with command line argument:
- Run choco install linkerd -s . --params="/path:<YOUR_PATH_HERE>"
  To run with environment variable:
- Run $env:linkerdPath = "<YOUR_PATH_HERE>" in powershell
  Uses default chocolatey install path if none of the above provided.
Modified package to accept command line args/ env variable for checksum
To run with command line argument:
- Run choco install linkerd -s . --params="/checksum:<VERSION_CHECKSUM>"
  (Keep params space seperated when specifying multiple)
  To run with environment variable:
- Run $env:linkerdCheckSum = "<VERSION_CHECKSUM>"
  If no checksum provided, default null value assigned and package ignores verification step
This package is compatible in a non-administrative environment but one must install chocolatey
in a non-administrative path.
(Steps explained here: https://chocolatey.org/docs/installation#non-administrative-install)

Further plans:
This PR is part of a set of planned PRs based on the discussions in
#3921
(specifically: #3921 (comment))

Signed-off-by: Animesh Narayan Dangwal animesh.leo@gmail.com

@ihcsim

This PR partially fixes linkerd#3063 by building a chocolatey package for Linkerd2's Windows CLI It adds the build scripts for the Linkerd chocolatey package and based on discussions in linkerd#3921 The package currently exists in the `bin/win` folder The `.nuspec` script has been modified for easy integration with the workflow done by @ihcsim here: https://github.com/ihcsim/linkerd-chocolatey To build outside of workflow environment (on windows system): - Modify `linkerd.nuspec` version tag to an available stable version of Linkerd2's cli eg: stable-2.7.0 - Install chocolatey - cd into `bin/win/` - Run `choco pack` - Run `choco install linkerd -s .` (make sure cmd/ powershell is running as administrator) - Run refreshenv to add `linkerd.exe` path to system - To unistall run `choco uninstall linkerd` Tackling requirements discussed in linkerd#3921 (specifically: linkerd#3921 (comment)) - Followed folder structure as described here: https://github.com/ihcsim/linkerd-chocolatey - Modified package to accept version through the metadata available in `.nuspec` file - Modified package to override install path with command line arg/ env variable To run with command line argument: - Run `choco install linkerd -s . --params="/path:<YOUR_PATH_HERE>"` To run with environment variable: - Run `$env:linkerdPath = "<YOUR_PATH_HERE>"` in powershell Uses default chocolatey install path if none of the above provided. - Modified package to accept command line args/ env variable for checksum To run with command line argument: - Run `choco install linkerd -s . --params="/checksum:<VERSION_CHECKSUM>"` (Keep params space seperated when specifying multiple) To run with environment variable: - Run `$env:linkerdCheckSum = "<VERSION_CHECKSUM>"` If no checksum provided, default null value assigned and package ignores verification step - This package is compatible in a non-administrative environment but one must install chocolatey in a non-administrative path. (Steps explained here: https://chocolatey.org/docs/installation#non-administrative-install) Further plans: This PR is part of a set of planned PRs based on the discussions in linkerd#3921 (specifically: linkerd#3921 (comment)) Signed-off-by: Animesh Narayan Dangwal <animesh.leo@gmail.com>

cpretzer · 2020-04-07T23:34:08Z

bin/win/tools/chocolateyinstall.ps1

+$version = $env:chocolateyPackageVersion
+$pp = Get-PackageParameters
+
+if ($pp['path'] -ne $null){


@drholmie thanks for this PR. There were a couple of failures in the tests:

Looks like we need a then statement here: https://github.com/koalaman/shellcheck/wiki/SC1050

Hi there! Thanks for reviewing the PR :) So since this is a PowerShell script, shellcheck isn't actually meant for linting this. Although thanks for catching this. I completely missed linting the code. For PowerShell scripts PSScriptAnalyzer can be used, doing so (for default values) I got the following output:

RuleName Severity ScriptName Line Message -------- -------- ---------- ---- ------- PSAvoidTrailingWhitespace Information chocolatey 33 Line has trailing whitespace install.ps 1 PSPossibleIncorrectComparisonWithNu Warning chocolatey 7 $null should be on the left side of equality ll install.ps comparisons. 1 PSPossibleIncorrectComparisonWithNu Warning chocolatey 18 $null should be on the left side of equality ll install.ps comparisons.

I'll make those changes however before adding to the PR, how should we go about adding PSScriptAnalyzer to the static checks of Linkerd? (if it is required to be added that is) or is it alright for now to just add ! -name win \ in the static_check.yml ?

this is a good question. I found a PSScriptAnalyzer GitHub Actions integration.

@kleimkuhler @alpeb Is this something that we want to integrate into github actions? The option is to ignore the .ps1 script, per the latest commit.

@cpretzer Definitely 👍 @drholmie can you take care of adding that as part of this PR? If you're not familiar with Github actions, I can take care of it as a followup PR.
Mind thought that'd be a separate effort from adding the chocolatey package into the Linkerd release assets, which I think should be tackled in yet another separate PR.

Thank you all for reviewing this! As mentioned in the draft PR comment (#3921 (comment)). It would be better to handle this in a separate PR. Considering the scope of this PR is meant to add the required files for building the choco package in the bin folder.

Regarding adding this to the release process, @ihcsim has already done some work on this as seen here.

cpretzer · 2020-04-07T23:45:37Z

bin/win/linkerd.nuspec

@@ -0,0 +1,32 @@
+<?xml version="1.0" encoding="utf-8"?>


shellcheck is parsing this file as a script, and we should add it to the list of ignored files in static_check.yml.

! -name *.nuspec \

Thanks for pointing this out! Will add it or ! -name win \ after resolving the previous comments.

Sounds good! Looking forward to getting this committed and merged.

Modified `chocolateyinstall.ps1` to follow the default rules in linter `PSScriptAnalyzer` Modified `static_checks.yml` to ignore `win` folder during shellcheck Signed-off-by: Animesh Narayan Dangwal <animesh.leo@gmail.com>

alpeb · 2020-04-15T21:28:22Z

@drholmie I was testing this and stumbled against this problem:

Can you please point us to some doc describing the versioning format to see what are our options?

kleimkuhler

Could some further instructions be provided on how users would build the CLI? Right now (as a non-native windows user) I had to follow along with the PR instructions. I would not have known what to do without that.

Also, I think I am following the directions correctly, I still get the following error:

❯ choco pack
Chocolatey v0.10.15
'stable-2.7.1' is not a valid version string.
Parameter name: version

This was after replacing ${LINKERD_VERSION} version field in linkerd.nuspec.

Modified `chocolateyinstall.ps1` to accept version string correctly Signed-off-by: Animesh Narayan Dangwal <animesh.leo@gmail.com>

drholmie · 2020-04-16T17:13:57Z

Thank you @alpeb @kleimkuhler for catching that! My apologies, I overlooked the acceptable formats for the version string. The format is just 2.7.1 not stable-2.7.1. I have fixed it in the latest commit, and have reflected the change in the original PR message.

Also @alpeb this is meant only for stable packages. The version tag, works based off what is provided in the .nuspec file. This makes it easier for Linkerd to release the choco package without relying on using chocolatey's community repo (as mentioned in the draft PR here: #3921 (comment)).

@kleimkuhler this package isn't meant to be built by the user. The intended workflow is that the user can download the choco package from the releases page and then run choco install linkerd-cli -s . to install the CLI.

kleimkuhler · 2020-04-16T17:47:58Z

@drholmie Oh okay I see the comment you referenced explaining this PR should just focus on adding the scripts, and follow-up should add it to CI (sorry for the lack of context here I had not seen the whole conversation on the original issue thread).

So with this merged, a manual step will still need to take place for updating the version, but then users can download the CLI with chocolatey?

drholmie · 2020-04-21T15:58:31Z

No worries @kleimkuhler :) At the moment, yes users would have to manually update the version, however as mentioned this is meant to be used with the work done by @ihcsim here. Where the end goal is to have the choco package in the releases page, from where windows users can download it and install it using chocolatey.

alpeb · 2020-04-22T16:00:38Z

Thanks for clearing things out @drholmie. Now I'm having this problem:

Any ideas what could be going on?

drholmie · 2020-04-22T16:21:10Z

Thanks @alpeb for catching that! My apologies, it looks like I missed a step in my original PR message. Two things are to be changed to make the package work on a regular system. First is specifying the version in version tag, the other is changing the file tag's source from bin\win\tools\** to tools\**. Have now reflected this in the original PR message.

alpeb · 2020-04-23T17:59:06Z

@drholmie Can you please explain why we need to make that path change on-the-fly, instead of changing it for good?

drholmie · 2020-04-24T15:31:59Z

Thanks @alpeb for reminding! (apologies I forgot to mention it in the previous comment). The files in this PR are meant to suit the work done by @ihcsim here: https://github.com/ihcsim/linkerd-chocolatey. The .nuspec is taken from there. Some files were modified to satisfy the requirements specified here: #3921 (comment). (The comment also mentions the future work required to develop the support for chocolatey).

alpeb

Ok, thanks @drholmie !
I understand now the path is bin\win\tools\ for when this is run by CI, and one has to change it to manually test the package building in a windows machine 👍
LGTM! 👍

cpretzer

nice work @drholmie !

drholmie · 2020-04-28T15:56:07Z

Thanks for all the help! And the approval :)

Followup to #4205 This adds: - A new job `psscript-analyzer` into the `statics_checks.yml` workflow for linting the Chocolatey Powershell script. - A series of new steps into the `gh_release` job of the `release.yml` workflow for updating the Chocolate spec file and generating the package. This is only triggered for stable releases. - The `Create release` step was updated to upload the generated package, if present.

Followup to #4205, supersedes #4205 This adds: - A new job psscript-analyzer into the `statics_checks.yml` workflow for linting the Chocolatey Powershell script. - A new `choco_pack` job in the `release.yml` workflow for updating the Chocolatey spec file and generating the package. This is only triggered for stable releases. It requires a windows runner in order to run the choco tooling (in theory it should have worked on a linux runner but in practice it didn't). - The `Create release` step was updated to upload the generated package, if present. - The source file path in `bin/win/linkerd.nuspec` was updated to make this work.

* CI steps for Chocolatey package - take 2 Followup to #4205, supersedes #4205 This adds: - A new job psscript-analyzer into the `statics_checks.yml` workflow for linting the Chocolatey Powershell script. - A new `choco_pack` job in the `release.yml` workflow for updating the Chocolatey spec file and generating the package. This is only triggered for stable releases. It requires a windows runner in order to run the choco tooling (in theory it should have worked on a linux runner but in practice it didn't). - The `Create release` step was updated to upload the generated package, if present. - The source file path in `bin/win/linkerd.nuspec` was updated to make this work. * Name nupkg file accordingly to the other release assets

drholmie force-pushed the master branch from 97c05da to f8ae38c Compare March 27, 2020 14:33

cpretzer suggested changes Apr 7, 2020

View reviewed changes

cpretzer reviewed Apr 7, 2020

View reviewed changes

drholmie requested review from alpeb and kleimkuhler as code owners April 15, 2020 06:45

Adding PR Feedback

05a2a9b

Modified `chocolateyinstall.ps1` to follow the default rules in linter `PSScriptAnalyzer` Modified `static_checks.yml` to ignore `win` folder during shellcheck Signed-off-by: Animesh Narayan Dangwal <animesh.leo@gmail.com>

drholmie force-pushed the master branch from ecd6ad6 to 05a2a9b Compare April 15, 2020 07:02

kleimkuhler reviewed Apr 16, 2020

View reviewed changes

Fixed version string usage

125b192

Modified `chocolateyinstall.ps1` to accept version string correctly Signed-off-by: Animesh Narayan Dangwal <animesh.leo@gmail.com>

drholmie force-pushed the master branch from fade6f2 to 125b192 Compare April 16, 2020 17:08

alpeb approved these changes Apr 27, 2020

View reviewed changes

cpretzer approved these changes Apr 27, 2020

View reviewed changes

cpretzer merged commit 7a560a7 into linkerd:master Apr 29, 2020

alpeb mentioned this pull request May 14, 2020

CI steps for Chocolatey package #4400

Closed

alpeb mentioned this pull request Jun 2, 2020

CI steps for Chocolatey package - take 2 #4536

Merged

Conversation

drholmie commented Mar 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alpeb commented Apr 15, 2020

Uh oh!

kleimkuhler left a comment

Choose a reason for hiding this comment

Uh oh!

drholmie commented Apr 16, 2020

Uh oh!

kleimkuhler commented Apr 16, 2020

Uh oh!

drholmie commented Apr 21, 2020

Uh oh!

alpeb commented Apr 22, 2020

Uh oh!

drholmie commented Apr 22, 2020

Uh oh!

alpeb commented Apr 23, 2020

Uh oh!

drholmie commented Apr 24, 2020

Uh oh!

alpeb left a comment

Choose a reason for hiding this comment

Uh oh!

cpretzer left a comment

Choose a reason for hiding this comment

Uh oh!

drholmie commented Apr 28, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

drholmie commented Mar 27, 2020 •

edited

Loading