enable: wpb-17321 refactor and fixes for wiab-demo #827
Conversation
mohitrajain
changed the title
sghosh23
left a comment
sghosh23
left a comment
There was a problem hiding this comment.
Hey @mohitrajain, please let me know if there are open questions from you based on my review.
| kube_config_path: "/home/{{ ansible_user }}/.kube/config" | ||
| iptables_rules_comment: "Wire Iptables Rules" | ||
| iptables_save_dir: "/home/{{ ansible_user }}/wire-iptables-rules" | ||
|
|
There was a problem hiding this comment.
I assume deploy_wiab.yml is the parent playbook who maintains the dependency to have a clean cluster deployment with necessary tools.
Clarification question:, Some tasks are with tag like minikube, cert_manager_networking. Can we run those tasks independently by using the tags?
OR it is meant to be run as a full deployment always?
There was a problem hiding this comment.
Hey, yes the playbook deploy_wiab.yml is the parent and interface to access the sub playbooks and also yes, user can individually run those set of tasks by using the appropriate tags. Explained all deployment flow and tags here:
https://github.com/wireapp/wire-server-deploy/blob/wpb-17321-fixes-ansible-wiab/offline/demo-wiab.md#deployment-flow
https://github.com/wireapp/wire-server-deploy/blob/wpb-17321-fixes-ansible-wiab/offline/demo-wiab.md#available-tags
By default, user can choose to let the deploy_wiab work with all tags and it will implement all sub-playbooks accordingly. However, based on user requirements, they can choose any sub-playbook to implement by using their tags.
| chain: PREROUTING | ||
| protocol: "{{ item.protocol }}" | ||
| jump: DNAT | ||
| in_interface: "{{ matching_bridge_interface }}" |
There was a problem hiding this comment.
May be a validation of matching_bridge_interface is a good idea
There was a problem hiding this comment.
I am trying to do it here: https://github.com/wireapp/wire-server-deploy/blob/wpb-17321-fixes-ansible-wiab/ansible/wiab-demo/deploy_wiab.yml#L72
Please confirm if this the same validation you are referring it
ansible/wiab-demo/install_pkgs.yml
Outdated
| state: directory | ||
| - name: Install yq-go | ||
| get_url: | ||
| url: "https://github.com/mikefarah/yq/releases/download/v4.48.1/yq_linux_amd64" |
There was a problem hiding this comment.
May be hardcoded version can be refactored as variable
There was a problem hiding this comment.
parameterized y-go version in install_pkgs playbook
| - name: Test HTTP connectivity from minikube node to its own public IP | ||
| command: > | ||
| docker exec {{ minikube_profile }} | ||
| sh -c "curl -sS -o /dev/null -w '%{http_code}' --connect-timeout 2 http://{{ wire_ip }}" |
There was a problem hiding this comment.
is it tested that 2 seconds timeout is good enough in a slow network?
May be we should test https too as 443 is configured
While using curl, i think we need to make sure the system will have curl installed.
There was a problem hiding this comment.
the test here is against the public IP on the same node. The request is leaving the docker (virt) network and then routed back to the host network stack via netfilter (iptables rules). It is not leaving the node. The idea here is to check if hairpin is required, once we know packets from docker network aren't able to access the public IP on the same node, we enable hairpin networking for both 80 and 443 ports.
curl is available in the minikube docker container by default.
| failed_when: false | ||
| changed_when: false | ||
|
|
||
| - name: Determine if hairpin NAT is required (treat any 1xx–5xx as success) |
There was a problem hiding this comment.
Why shall we treat 4xx-5xx as success? If you are just testing connectivity regardless of the status code, please reflect it on the task name.
There was a problem hiding this comment.
This is just to confirm if public IP is available - a simple TCP handshake is also enough could have used something simple like netcat but wasn't sure if nc would be available in minikube docker container. The task is confusing, let me make it more clear and switch using netcat.
Co-authored-by: Sukanta <[email protected]>
Co-authored-by: Sukanta <[email protected]>
…pdate wire_values accoordingly
…cret management to it
… tasks for minikube networking
sghosh23
left a comment
sghosh23
left a comment
There was a problem hiding this comment.
Please sonarCloud code analysis complains ;)
| with_items: "{{ random_strings.results }}" | ||
|
|
||
| - name: Check if ZAUTH Docker image already exists | ||
| shell: docker images | grep "quay.io/wire/zauth" | awk '{print $1":"$2}' | head -1 |
There was a problem hiding this comment.
Instead of using grep, docker inspect could be more fitting here!
There was a problem hiding this comment.
we are searching for the image first, right?
| - { name: 'zrest', length: 64 } | ||
| - { name: 'minio_access_key', length: 20 } | ||
| - { name: 'minio_secret_key', length: 42 } | ||
|
|
There was a problem hiding this comment.
Don't we need minio_cargohold_access_key and minio_cargohold_secret_key
There was a problem hiding this comment.
I have added a logic to update default access_key and secret_key for minio
ansible/wiab-demo/wire_secrets.yml
Outdated
|
|
||
| - name: Update secrets for coturn | ||
| shell: | | ||
| yq eval -i '.secrets.zrestSecrets += ["{{ zrest }}"]' "{{ ansible_user_dir }}/wire-server-deploy/values/coturn/secrets.yaml" |
There was a problem hiding this comment.
why using =+ ? Won't it append the secret in the next run?
There was a problem hiding this comment.
using =+ as it is a list and it won't overwrite the secrets as the playbook is idempotent and it won't update the secrets
ansible/wiab-demo/wire_secrets.yml
Outdated
| block: | ||
|
|
||
| - name: Prepare secrets.yaml files for the demo environment | ||
| shell: source {{ ansible_user_dir }}/wire-server-deploy/bin/wiab-demo/offline_deploy_k8s.sh && BASE_DIR={{ ansible_user_dir }}/wire-server-deploy process_charts demo secrets |
There was a problem hiding this comment.
I think you can remove BASE_DIR={{ ansible_user_dir }}/wire-server-deploy when sourcing the script, its already in the env var
| zauth_private: "{{ zauth_output.stdout_lines[1].split()[1] }}" | ||
|
|
||
| - name: Created following secrets | ||
|
|
There was a problem hiding this comment.
Overall, shall we set no-log: true to make sure secrets does not get exposed to the console?
There was a problem hiding this comment.
This has been removed, it is negative changelog. We aren't logging any secrets to console.
…ASE_DIR in wire_values
…d added minio creds
… file permissions error
…using tags in deploy_wiab and update the documentation
sghosh23
left a comment
sghosh23
left a comment
There was a problem hiding this comment.
I took a look, can't inspect all the changes you made after my comments, seen some are cleaned up and some are refactored which is really good. Let's merge!
|
* fix: wpb-17321 fix coturn secrets for demo-wiab * fix: wpb-17321 fix postgresql secrets for demo-wiab * enable: wpb-17321 kube-prometheus-stack values and enabled monitoring support from wire-server * enable: wpb-17321 add values for wire-utility in demo-wiab * enable: wpb-17321 sync serviceMonitor for ingress-nginx-controller for prod to disable monitoring * enable: wpb-17321 add changelogs * enable: wpb-17321 refactor and fixes for wiab-demo (#827) * enable: wpb-17321 refactor and fixes for wiab-demo, added all changes in changelog * fix: create a new tag for wire_secrets and handle errors from zauth wpb-17321 * fix wpb-17321: made wire_secrets playbook idempotent * fix wpb-17321: handle wire-utility deploy issues and update documentation for demo-wiab * Update changelog.d/3-deploy-builds/demo-wiab-ansible-fixes Co-authored-by: Sukanta <[email protected]> * Update changelog.d/3-deploy-builds/demo-wiab-ansible-fixes Co-authored-by: Sukanta <[email protected]> * fix wpb-17321: parameterize y-go version in install_pkgs playbook * fix wpb-17321: change http check to netcat based check in hairpin networking * fix wpb-17321: refactor offline_deploy_k8s.sh to work with envs and update wire_values accoordingly * fix wpb-17321: refactor wire_secrets to be idempotent and move all secret management to it * fix wpb-17321: when conditions in deploy_wiab to better manage common tasks for minikube networking * fix wpb-17321: cert-manager deploy control with cert_manager_networking tag * fix wpb-17321: update the documentation for demo-wiab * fix wpb-17321: added minio secrets in demo-values and removed extra BASE_DIR in wire_values * fix wpb-17321: move away from yq-go to ansible native yaml updates and added minio creds * fix wpb-17321: fix basc script suggestions from sonarcloud * fix wpb-17321: fix wire_secrets for cargohold * fix wpb-17321: fix wire_secrets for fake-aws-s3 * fix wpb-17321: fix clean_cluster permissions * fix: wpb-17321 add coturn empty values file, ignore the download task file permissions error * fix: wpb-17321 fix wire_secrets for non-prepared secrets, fixed flow using tags in deploy_wiab and update the documentation * Wpb 17321 enable demo cd (#828) * enable: wpb-17321 add terraform and bash scripts to enable cd-demo * enable: wpb-17321 added changelog and a note to old wiab-staging scripts * fix: wpb-17321 remove the debugging statements from cd-demo.sh --------- Co-authored-by: Sukanta <[email protected]> * fix: wpb-21356 suggestions highligted by shellcheck.sh linting * fix: wpb-21356 fix the when condition for wire_values and cert manager task * Update cd_demo.sh to test cert_manager deployment --------- Co-authored-by: Sukanta <[email protected]>
Fixed: ansible playbooks for demo-wiab for the tags
Fixed: ansible playbooks for demo-wiab for the directory usage
Changed: refractor deploy_wiab to have common inventory together and stop asset service post deploy
Added: hairpin_networking check and rules placement for demo_wiab deployments
Changed: refactor helm_install to have separate values processing and enable wire-utility deployment
Fixed: dependecy on yq-go and making docker installation idempotent
Changed: refactor iptables_rules, add additional checks for system and fix port ranges for calling
Added: added a check for SPF TXT record in verify_dns
Changed: docker load logic for zuath image, containers_adminhost and fixed expected kube_config path
Added: a sperate playbook and tag to allow bringing old values in a demo_wiab deloyment
Fixed: calling components deployment and archiving old values when processing them
Changed: enable deployment of smtp, postgresql and kube-prometheus-stack, update artifact hash
Change type
Basic information
Testing
Tracking
changelog.dKnowledge Transfer
Motivation
Objective
Reason
Use case