Skip to content

Make cephadm_key module stateless #145

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

Merged
merged 3 commits into from
Jul 29, 2024
Merged

Make cephadm_key module stateless #145

merged 3 commits into from
Jul 29, 2024

Conversation

m-bull
Copy link
Contributor

@m-bull m-bull commented May 9, 2024

This module should never write to files because they always exist inside the cephadm container, which
is ephemeral. This change removes all file-writing functions and references to keyrings.

This fixes the following failure:

The full traceback is:                                                                                                                                                                                             
  File "/tmp/ansible_cephadm_key_payload_51gs4a3l/ansible_cephadm_key_payload.zip/ansible/module_utils/basic.py", line 688, in selinux_context                                                                     
    ret = selinux.lgetfilecon_raw(to_native(path, errors='surrogate_or_strict'))                                                                                                                                   
  File "/tmp/ansible_cephadm_key_payload_51gs4a3l/ansible_cephadm_key_payload.zip/ansible/module_utils/compat/selinux.py", line 95, in lgetfilecon_raw                                                             
    rc = _selinux_lib.lgetfilecon_raw(path, byref(con))                                                  
  File "/tmp/ansible_cephadm_key_payload_51gs4a3l/ansible_cephadm_key_payload.zip/ansible/module_utils/compat/selinux.py", line 23, in _check_rc                                                                   
    raise OSError(errno, os.strerror(errno))
failed: [oscephpor01] (item={'name': 'client.cinder', 'caps': {'mon': 'profile rbd', 'osd': 'profile rbd pool=volumes, profile rbd pool=vms, profile rbd-read-only pool=images', 'mgr': 'profile rbd pool=volumes, profile rbd pool=vms'}}) => changed=false                                                                
  ansible_loop_var: item                                                                                 
  invocation:                                                                                            
    module_args:                                                                                         
      attributes: null                                                                                   
      caps:                                                                                              
        mgr: profile rbd pool=volumes, profile rbd pool=vms                                              
        mon: profile rbd                                                                                 
        osd: profile rbd pool=volumes, profile rbd pool=vms, profile rbd-read-only pool=images           
      dest: /etc/ceph/                                                                                   
      group: null                                                                                        
      import_key: true                                                                                   
      mode: null                                                                                         
      name: client.cinder                                                                                                                                                                                          
      output_format: json                           
      owner: null                                                                                                                                                                                                  
      secret: ''                                    
      selevel: null                                                                                      
      serole: null                                                                                       
      setype: null                                                                                                                                                                                                 
      seuser: null                                                                                       
      state: present                                
      unsafe_writes: false                        
  item:                                                                                                  
    caps:                                                                                                
      mgr: profile rbd pool=volumes, profile rbd pool=vms                                                
      mon: profile rbd                              
      osd: profile rbd pool=volumes, profile rbd pool=vms, profile rbd-read-only pool=images             
    name: client.cinder                             
  msg: path /etc/ceph/ceph.client.cinder.keyring does not exist                                          
  path: /etc/ceph/ceph.client.cinder.keyring

Which is caused by module.set_fs_attributes_if_different running on the host when the keyring file is created in a previous step in an ephemeral cephadm container, and no longer exists.

This fixes a long-standing idempotency issue, where ceph keys can be created but the module fails on subsequent invocations. A side-effect of this change: the ability to specify and generate a secret string has been removed, and users should rely on retrieving the secret key from the cluster directly by registering the output from cephadm_key tasks.

@m-bull m-bull force-pushed the fix/cephadm-keys branch from b798d56 to 2e98b87 Compare May 9, 2024 18:00
@m-bull m-bull marked this pull request as ready for review May 9, 2024 18:17
@m-bull m-bull requested a review from a team as a code owner May 9, 2024 18:17
Copy link
Member

@mnasiadka mnasiadka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll probably need some changelog entry that generate_secret and fetch_initial_keys are not a thing anymore

@m-bull m-bull force-pushed the fix/cephadm-keys branch 2 times, most recently from f4ff75a to 5de4180 Compare May 10, 2024 08:45
@m-bull
Copy link
Contributor Author

m-bull commented May 10, 2024

Thanks @mnasiadka - updated the changelog and role docs. I'm not sure fetch_initial_keys ever actually did anything, but we've deprecated it now anyway!

Alex-Welsh
Alex-Welsh previously approved these changes May 14, 2024
m-bull and others added 2 commits July 29, 2024 12:52
This module should never write to files because they
always exist inside the cephadm container, which
is ephemeral. Remove all file-writing functions and
references to keyrings.
Co-authored-by: Alex-Welsh <[email protected]>
@cityofships
Copy link
Member

I took the liberty of rebasing this

cityofships
cityofships previously approved these changes Jul 29, 2024
@cityofships
Copy link
Member

cityofships commented Jul 29, 2024

Ideally needs a version bump in galaxy.yml too

@cityofships cityofships merged commit 432d907 into master Jul 29, 2024
2 checks passed
@cityofships cityofships deleted the fix/cephadm-keys branch July 29, 2024 18:42
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.

4 participants