-
Notifications
You must be signed in to change notification settings - Fork 73
maas dns role for configuring maas dns domains and records #778
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?
Conversation
Please add a README explaining all of the variables and what the secrets variables should look like. Like this: https://github.com/ceph/ceph-cm-ansible/blob/main/roles/nameserver/README.rst |
9a88e24
to
5fed40f
Compare
If you look at the README in the Web UI, it does not get rendered as expected. Maybe it's |
1eb5687
to
92b9df3
Compare
https://github.com/ceph/ceph-cm-ansible/tree/wip-maas-dns/roles/maas_nameserver |
152f202
to
b375ae2
Compare
roles/maas_nameserver/README.md
Outdated
The `maas_nameserver` role depends on the `secrets` role to load encrypted MAAS API credentials. The secrets file is expected at `{{ secrets_path }}/maas.yml`, where `secrets_path` defaults to `/etc/ansible/secrets`. | ||
|
||
### Example Secrets File | ||
Create and encrypt the file (e.g., `/etc/ansible/secrets/maas.yml`): |
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.
This should be {{ secrets_path }}
with no mention of /etc/ansible
.
How feasible is it to test this against our existing ansible inventories? |
I have tested and verified it against a similar inventory like we have in sepia |
94a08ae
to
9a6a76d
Compare
6c1e94d
to
815e3e5
Compare
roles/maas_nameserver/vars/main.yml
Outdated
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.
dns_domains should be empty in this repo and overridden in the ansible inventory.
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.
Fixed and updated the changes in the readme file
51d3b98
to
6264d86
Compare
Ping me and @dmick when this is ready again for review please |
fa552bf
to
0701de9
Compare
roles/maas_nameserver/vars/main.yml
Outdated
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 this necessary?
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 removed it for now
654665d
to
b346a19
Compare
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.
Stopped midway through when I got to something I could not begin to explain. We need to have a live session about this.
roles/maas_nameserver/README.md
Outdated
|
||
## Dependencies | ||
|
||
- **Secrets File**: A `maas.yml` file at `{{ secrets_path }}/maas.yml` provides MAAS API credentials (e.g., `maas_api_key`, `maas_api_url`). No separate secrets role is required; credentials are loaded via `include_vars`. |
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 assume it literally needs to contain maas_api_key and maas_api_url; those aren't 'examples', but: why is the api_url a secret? and can you show a sample {{ secrets_path }}/maas.yml file just to be clear?
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 agree that the url it's not a secret but I think it's a good practice to place the api_key, profile and url in the same place since we need all three to communicate with the maas api for maas.yml there is already an example in line 64
roles/maas_nameserver/README.md
Outdated
|
||
## Usage | ||
|
||
1. **Place the Role** |
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.
"Place" isn't the verb I'd choose, but I'd just remove this whole step; it's obvious that to use code you have to have it in place
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.
Removed it
roles/maas_nameserver/README.md
Outdated
|
||
```ini | ||
[maas] | ||
maas.internal.ceph.ibm.com ip=10.11.120.237 |
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.
Since I now know there are multiple hosts involved in the maas installation, it seems like we need to specify which host is getting the DNS update. The hosts Fernando talks about are "db", "region+rack", and "secondary rack". How are these specified in inventory, and which one should execute this role?
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 don't know of other hosts I am executing the role against the host running the maas api
roles/maas_nameserver/README.md
Outdated
[maas] | ||
maas.internal.ceph.ibm.com ip=10.11.120.237 | ||
|
||
[snipeit] |
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 this a target host in some way? This role doesn't involve snipe directly, does it?
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.
No, it's just an example
roles/maas_nameserver/README.md
Outdated
[snipeit] | ||
snipe-it.internal.ceph.ibm.com ip=10.60.100.11 | ||
|
||
[machine] |
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.
What are the contents of the machine group supposed to be?
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 just an example, it doesn't mean anything, I could call it anything I just chosen some group name
- "maas" | ||
|
||
# List of target hosts for DNS records, excluding the 'maas' group by default. | ||
# Can be overridden in secrets/maas.yml (e.g., target_hosts: ['host1.example.com', 'host2.example.com']) |
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.
not a secret
roles/maas_nameserver/tasks/main.yml
Outdated
ansible.builtin.command: "maas login {{ maas_profile }} {{ maas_api_url }} {{ maas_api_key }}" | ||
register: maas_login | ||
changed_when: maas_login.rc == 0 | ||
failed_when: maas_login.rc != 0 and maas_login.stderr != "Profile already exists" |
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.
When does login say "Profile already exists"? It always already exists for you to be able to login, doesn't it?
roles/maas_nameserver/tasks/main.yml
Outdated
ansible.builtin.set_fact: | ||
dns_records: [] | ||
|
||
# Define target hosts list for DNS records, excluding the 'maas' group. |
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.
if the group to be excluded is in a variable, the comment should refer to the variable
roles/maas_nameserver/tasks/main.yml
Outdated
|
||
# After population, target_hosts will contain all hosts from inventory groups except 'maas', e.g., | ||
# ['machine001.internal.example.com', 'snipe.internal.example.com', 'vm-14.example.com']. | ||
# This can be overridden in secrets/maas.yml (e.g., target_hosts: ['host1.example.com', 'host2.example.com']) |
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.
not a secret
roles/maas_nameserver/tasks/main.yml
Outdated
- name: Build DNS records for all interfaces | ||
ansible.builtin.set_fact: | ||
dns_records: "{{ dns_records + [{'name': item[0].split('.')[0], 'ip': interface_ip, 'type': 'A', 'domain': item[1].value}] }}" | ||
loop: "{{ (target_hosts | default([])) | product(dns_domains | dict2items) | list }}" |
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 don't have the faintest idea what's going on here. It looks as though this will make a list of two-element combinations of every item in target_hosts and every item in dns_domains. How is that remotely useful?
I think at this point I'm going to have to stop reviewing until we can talk. I'll save this review this far, but I'm not done.
26897f8
to
b42e556
Compare
25ca9ca
to
7dfab8b
Compare
ad35b54
to
e1ba19c
Compare
I think you want to squash your commits. Also, effort should be made to keep the actual commit message (the top line) as short as possible. e.g., |
- Configures DNS entries for hosts (e.g., main interfaces, IPMI interfaces) using `dns_domains` and inventory variables (`ip`, `ipmi`). - Dynamically sets `maas_cluster_instance.host` via `maas_api_url` in `defaults/main.yml`, with credentials (`customer_key`, `token_key`, `token_secret`) loaded from a secrets file (`secrets/maas.yml`). - Filters inventory to process only hosts with `ip` or `ipmi` variables, excluding groups like `all` and `ungrouped`. - Cleans up unwanted DNS records and domains not in `dns_domains` or `excluded_domains`, skipping default MAAS domains. - Skips NS record creation due to module limitations, with instructions for manual creation via MAAS CLI/UI. - Includes comprehensive `README.md` with: - Inventory example with `mac`, `ip`, `ipmi`, and `bmc` attributes (e.g., `server01.example.com mac=00:1a:2b:3c:4d:5e ip=192.168.1.11 ipmi=192.168.2.11 bmc=00:1a:2b:3c:4d:5f`). - Setup instructions for inventory, secrets, and variables. - Troubleshooting for common issues (e.g., network errors, missing `dns_domains`). - Performance tips (e.g., fact caching). - Ensures idempotency and supports large inventories.
No description provided.