-
Notifications
You must be signed in to change notification settings - Fork 44
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
Wiab automation #728
Wiab automation #728
Conversation
…nd nftables hetzner firewall
INGRESSNODE=$(d kubectl get pods -l app.kubernetes.io/name=ingress-nginx -o=custom-columns=NODE:.spec.nodeName --no-headers) | ||
d kubectl cordon "$INGRESSNODE" | ||
|
||
wget https://charts.jetstack.io/charts/cert-manager-v1.13.2.tgz |
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.
is there a reason we manually download and unpack charts instead of using helm repo add
?
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.
Taken from https://github.com/wireapp/wire-server-deploy/blob/master/offline/docs_ubuntu_22.04.md
I guess the original thought was to avoid creating external dependencies for a "true" offline install, without internet. Which does not work anyway with (automated) demo WIAB, but I wanted to avoid upstreaming changes to the "official" offline documentation.
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 think we can remove this manual pulling of cert-manager helm chart in another pr and use our new helm-charts / wire-builds repo to put in the offline artifact.
fi | ||
if [ "$DO_SYSTEM_CLEANUP" = true ] && [ "$FORCE_REDEPLOY" = 0 ]; then |
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.
thought: is there a reason to avoid an else
branch and instead duplicate the conditions in the following if
s?
fi | |
if [ "$DO_SYSTEM_CLEANUP" = true ] && [ "$FORCE_REDEPLOY" = 0 ]; then | |
elif [ "$FORCE_REDEPLOY" = 0 ]; then |
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 existing logic accounts for the case of a "clean" target system where no resource removal is necessary, in which we don't need to jump into the system_cleanup() function. Otherwise the output looks like this:
INFO: Target system clean, no previous wire-server-deploy installation found.
INFO: Cleaning up all VMs, docker resources and wire-server-deploy files on wiab-autodeploy.wire.link.
It's just cosmetics, but I's like to have consistent script output.
set -eo pipefail; | ||
|
||
#cassandra | ||
ufw allow 9042/tcp; |
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.
thought: why do we use Ansible at all if we use imperative provisioning everywhere?
suggestion: use a suitable Ansible module to manage firewall rules.
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 existing playbooks are supposed to take care of that AFAIK, which did not work reliably in the past - some deployments did have the rules applied, some did not.
I admit it's hacky, but it works reliably, and actual customer site deployments use their own networking / firewalling.
While trying to run it locally for an installation, got the following error -
|
Hmm I think that's a nix / ansible issue. I'm allready explicitly setting There's more discussion here NixOS/nixpkgs#223151 |
bin/autodeploy.sh
Outdated
--force-redeploy Force cleanup of previous Wire-in-a-box installation on target host | ||
--artifact-hash String, specifies artifact ID as created here: https://github.com/wireapp/wire-server-deploy/actions/workflows/offline.yml | ||
Defaults to 5c06158547bc57846eadaa2be5c813ec43be9b59 | ||
--target-system String, domain name used to access the target host |
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 think it would be good if this parameter contained the "domain" string somewhere, I initially expected it was like the OS version.
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.
Good point, I've renamed the --target-system
flag to --target-domain
.
Quality Gate passedIssues Measures |
https://wearezeta.atlassian.net/browse/WPB-9381