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

Add script to install the latest version of jsctl #76

Merged
merged 1 commit into from
Jan 17, 2023
Merged

Conversation

inteon
Copy link
Contributor

@inteon inteon commented Jan 16, 2023

This is how we recommend the user installs jsctl currently:

$ VERSION="0.1.15"
$ ASSET_REF="jsctl-$VERSION-$(uname -s)-$(uname -m)"
$ ARCHIVE="$ASSET_REF.tar.gz"
$ curl -LO https://github.com/jetstack/jsctl/releases/download/v$VERSION/$ARCHIVE
$ tar -zxvf $ARCHIVE
$ sudo mv ./$ASSET_REF/jsctl /usr/local/bin/jsctl

This does not work for me since uname -s returns Linux instead of linux and that causes the sudo mv command to fail.

In #51 we were saying not to create a one-line install script, however I'm convinced that the advantages (simple & easy-to-use) outweigh the disadvantages (potentially insecure -> which is a moot point since the script is used to download and run our binary on the machine).

I renamed the release artefacts such that they don't contain the version anymore, this makes it easier to download the latest version directly. I also removed the architecture renaming from .goreleaser.yml, that is now done in the script instead. I also removed some target architectures that I think are not used (anymore) like windows 32 bit etc.

After creating a new release, the one liners should be:

curl -fsSL https://raw.githubusercontent.com/jetstack/jsctl/main/install.sh | sh

and

irm https://raw.githubusercontent.com/jetstack/jsctl/main/install.ps1 | iex

Copy link
Contributor

@irbekrm irbekrm left a comment

Choose a reason for hiding this comment

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

Thanks for working on this @inteon !

I agree with the general approach of having an install script and changing the file names make sense for me.
I think we want to keep Linux arm builds though, see my comment- let me know what you think

install.sh Outdated
target_file="jsctl"
fi

jsctl_uri="https://github.com/jetstack/jsctl/releases/download/latest/${target}.tar.gz"
Copy link
Contributor

Choose a reason for hiding this comment

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

Note: in this case returning the latest release should be what we always want as there are no patch releases for non-latest minor/major releases.

.goreleaser.yml Show resolved Hide resolved
install.ps1 Outdated Show resolved Hide resolved
Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com>
Copy link
Contributor

@irbekrm irbekrm left a comment

Choose a reason for hiding this comment

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

Thanks for working on this @inteon !

I've played around with the script a bit, all seems to work as expected 👍🏼

Happy to review any follow-up fixes if needed.

/lgtm

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