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

Rename host group names for valid Ansible names #904

Merged
merged 1 commit into from
Oct 11, 2023

Conversation

mkllnk
Copy link
Member

@mkllnk mkllnk commented Oct 11, 2023

Ansible was complaining:

[WARNING]: Invalid characters were found in group names but not replaced, use -vvvv to see details

Dashes are not allowed in group names like au-prod.

This requires an update of the staging servers. Last time we tried it, we ran into:

We need to update the deploy scripts for staging after merging this:

# No, don't do this:
#ansible-playbook playbooks/setup_semaphore_deployment.yml -l au_staging,fr_staging,uk_staging -t provision_deploy_script

# This sets the ansible_limit variable correctly:
ansible-playbook playbooks/setup_semaphore_deployment.yml -l au_staging -t provision_deploy_script
ansible-playbook playbooks/setup_semaphore_deployment.yml -l fr_staging -t provision_deploy_script
ansible-playbook playbooks/setup_semaphore_deployment.yml -l uk_staging -t provision_deploy_script

Ansible was complaining:
```
[WARNING]: Invalid characters were found in group names but not replaced, use -vvvv to see details
```

Dashes are not allowed in group names like au-prod.
@mkllnk mkllnk self-assigned this Oct 11, 2023
@dacook
Copy link
Member

dacook commented Oct 11, 2023

That's interesting, it seems like a clever hack that the limit param is copied into the script:

https://github.com/openfoodfoundation/ofn-install/blob/master/roles/semaphore_deployment/templates/deploy.j2#L36

I wonder if it's even necessary; I would think that --connection local ensures it only runs locally.

@dacook dacook merged commit 26903ad into openfoodfoundation:master Oct 11, 2023
2 checks passed
@mkllnk mkllnk deleted the group-names branch October 11, 2023 05:18
@mkllnk
Copy link
Member Author

mkllnk commented Oct 11, 2023

My command was actually not right. It inserted all three hosts into the command on all servers. But that will test if the local connection is restricting correctly. In that case we could get rid of that parameter.

@mkllnk
Copy link
Member Author

mkllnk commented Oct 11, 2023

So, I ran a deploy on au-staging and got a "failure":

PLAY RECAP *********************************************************************
staging.coopcircuits.fr    : ok=31   changed=16   unreachable=0    failed=0    skipped=6    rescued=0    ignored=0   
staging.openfoodnetwork.org.au : ok=9    changed=2    unreachable=0    failed=1    skipped=1    rescued=1    ignored=0   
staging.openfoodnetwork.org.uk : ok=9    changed=5    unreachable=0    failed=1    skipped=1    rescued=1    ignored=0   

It tried to deploy the local machine (au-staging) three times in parallel. Removing old releases failed on a conflict. One host reported success and the others failed. It didn't connect to fr or uk though. If we didn't have the limit in, it would try to execute the deploy script as many times as we have hosts.

@dacook
Copy link
Member

dacook commented Oct 11, 2023

Thanks for checking that out, now we know for sure! We should document how to run this playbook somewhere.

@dacook
Copy link
Member

dacook commented Oct 16, 2023

I just had an idea: rather than ansible_limit, we should be able to reference the current hostname.

But hopefully we won't need to use it again for a long time, so I won't bother trying it now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants