-
-
Couldn't load subscription status.
- Fork 126
Add golink LXC #906
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
base: main
Are you sure you want to change the base?
Add golink LXC #906
Conversation
|
We don't use git pull way of deploying app is very risky as you will pull possibly unstable code and this would result in people possibly losing their data or w/e. |
|
Hi @tremor021, Usually I would agree to stick with tagged releases but the codebase that the script pulls from is maintained by Tailscale themselves so there's a high degree of confidence that the main branch will be stable. Setting it up this way allows the go build system to build the latest version on each run straight from the source which would be more trusted/open than relying on a pre-built binary. |
|
@SimplyMinimal No matter what Compandy is behind it, we don't do git pull, only latest tagged release via our fetch_and_deploy_gh_release func. |
install/golink-install.sh
Outdated
| setup_go | ||
|
|
||
| msg_info "Cloning Golink Repository" | ||
| $STD git clone https://github.com/tailscale/golink.git /opt/golink |
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.
block
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.
I've swapped it with a curl to a tagged release instead.
@SimplyMinimal the point i'm making is that we need to have something to "latch" on as we deploy this. We decided that it would be a tagged release. Pulling code from a "live" repo is risky, no matter whos maintaining it. |
|
@SimplyMinimal I saw they have a 1.0.0 Tag, you should be able to use that. |
|
I attempted to use variations of Do you have any suggestions on how to best pull the tarball? Is using curl to fetch the tagged archive ok or does it need to be within the For clarity, here is the last example I've tried: Error: |
|
In the meantime, I would postpone the PR. If necessary, release the correct tarballs. Then the problem will be solved. |
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.
some things to match our standards, thanks!
| cd /opt/golink || exit | ||
| $STD go mod tidy | ||
|
|
||
| # Clean Go cache before building to save space |
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.
remove
| # Clean Go cache before building to save space | ||
| $STD go clean -cache -modcache | ||
|
|
||
| # Build with optimizations to reduce space usage |
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.
remove
|
|
||
| # Clean Go cache before building to save space | ||
| $STD go clean -cache -modcache | ||
|
|
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.
remove
| msg_info "Building Golink" | ||
| cd /opt/golink || exit | ||
| $STD go mod tidy | ||
|
|
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.
remove
|
|
||
| # Clean up build artifacts |
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.
remove
| systemctl stop golink | ||
| msg_ok "Stopped $APP" | ||
|
|
||
| msg_info "Updating $APP" |
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.
don't use a var 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.
Just asking as a learning experience here as this is my first contribution to this project. Based on the other scripts in the repo, almost all of them makes use of the $APP variable which makes sense. You'd have one single place to update the app name across the whole script and it makes it more obvious for readability.
I see several comments here not to use the $APP variable and suggests instead to hardcode the app name. Could I ask why?
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.
It's something we have started doing recently, there were some using a var some without, to make it easier to read and more simple to debug we chose to rather not use a var here as it doesn't really add any value here. More stylistic we know, but this ensures all scripts are line.
| chmod +x golink | ||
| RELEASE=$(git describe --tags --always 2>/dev/null || echo "main-$(git rev-parse --short HEAD)") | ||
| echo "${RELEASE}" >/opt/${APP}_version.txt | ||
| msg_ok "Updated $APP to ${RELEASE}" |
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.
same here
| echo "${RELEASE}" >/opt/${APP}_version.txt | ||
| msg_ok "Updated $APP to ${RELEASE}" | ||
|
|
||
| msg_info "Starting $APP" |
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.
| msg_info "Starting $APP" | |
| msg_info "Starting Service" |
|
|
||
| msg_info "Starting $APP" | ||
| systemctl start golink | ||
| msg_ok "Started $APP" |
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.
same here
| msg_ok "Started $APP" | ||
| msg_ok "Update Successful" | ||
| else | ||
| msg_ok "No update required. ${APP} is already up to date" |
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.
same here
🛑 New scripts must first be submitted to ProxmoxVED for testing.
PRs for new scripts that skip this process will be closed.
✍️ Description
Added golink from Tailscale as LXC container
🔗 Related PR / Issue
Link: #N/A
✅ Prerequisites (X in brackets)
🛠️ Type of Change (X in brackets)
README,AppName.md,CONTRIBUTING.md, or other docs.🔍 Code & Security Review (X in brackets)
Code_Audit.md&CONTRIBUTING.mdguidelinesAppName.sh,AppName-install.sh,AppName.json)📋 Additional Information (optional)