Skip to content

Conversation

@stejskalleos
Copy link
Contributor

No description provided.

@stejskalleos stejskalleos changed the title Module & role for generating registration commands [WIP] Module & role for generating registration commands Sep 8, 2021
Copy link
Contributor

@wbclark wbclark left a comment

Choose a reason for hiding this comment

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

interesting concept @stejskalleos ... thanks for the PR!

understanding that this is a WIP, right now the role looks like a wrapper that provides the same features as the module itself. thanking about what users might like to see from that, how about registering the generated commands in a variable (so that other ansible may refer to it) and giving the role options to output the generated commands in the terminal and/or write them into a file or script on the ansible target host.

let us know please what you envision in terms of what it will look like for users to use this... looking forward to hearing from you

@stejskalleos stejskalleos changed the title [WIP] Module & role for generating registration commands [WIP] Module for generating registration command Sep 9, 2021
@stejskalleos
Copy link
Contributor Author

right now the role looks like a wrapper that provides the same features as the module itself

Yeah, after some thinking I decided to remove the role. This module will be part of another role for converting centos machines to rhel (WIP), so having role just for generating registration commands doesn't make sense.

@wbclark
Copy link
Contributor

wbclark commented Sep 9, 2021

Yeah, after some thinking I decided to remove the role. This module will be part of another role for converting centos machines to rhel (WIP), so having role just for generating registration commands doesn't make sense.

Understood. Since you don't need the role right now for your particular goal, but the basic core of a role is still present in the commit history: I could use what is there to create a separate WIP PR so that it's not lost if this gets squashed on merge, meanwhile you would not need to write additional tests for the role, unrelated to your actual goal.

So as to not bog down this PR with further discussion that's not related to your immediate goal, I created an issue #1284 to discuss how a primary Ansible user should interact with the global registration feature. That said, please feel free to weigh in if you have any ideas about how that should work.

@wbclark
Copy link
Contributor

wbclark commented Sep 9, 2021

Also created #1285 so the initial work on a role is not lost -- thanks @stejskalleos for that

Copy link
Contributor

@wbclark wbclark left a comment

Choose a reason for hiding this comment

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

Am I correct in understanding that global registration should use only the Foreman API? So tests/fixtures/apidoc/registration_command.json should work as a symlink to foreman.json instead of a separately populated file?

@stejskalleos stejskalleos changed the title [WIP] Module for generating registration command Module for generating registration command Sep 10, 2021
@stejskalleos
Copy link
Contributor Author

Thanks for creating #1285,
PR updated, I removed files that shouldn't be part of the commit andtests/fixtures/apidoc/registration_command.json is link to foreman.json.

@evgeni evgeni requested a review from wbclark September 14, 2021 14:57
DOCUMENTATION = '''
---
module: registration_command
version_added: 1.0.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
version_added: 1.0.0
version_added: 2.3.0

Comment on lines +63 to +80
organization=dict(type='entity'),
location=dict(type='entity'),
hostgroup=dict(type='entity'),
operatingsystem=dict(type='entity'),
smart_proxy=dict(type='entity'),
insecure=dict(type='bool'),
setup_remote_execution=dict(type='bool'),
setup_insights=dict(type='bool'),
packages=dict(type='str'),
update_packages=dict(type='bool'),
repo=dict(type='str'),
repo_gpg_key_url=dict(type='str'),
jwt_expiration=dict(type='int', default=4),
remote_execution_interface=dict(type='str'),
lifecycle_environment=dict(type='entity', flat_name='lifecycle_environment_id'),
ignore_subman_errors=dict(type='bool'),
force=dict(type='bool'),
activation_keys=dict(type='list', elements='str'),
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please document these in the DOCUMENTATION string above, under options:? The activation key module has a nice example of what it looks like:

options:
name:
description:
- Name of the activation key
required: true
type: str
description:
description:
- Description of the activation key
type: str
lifecycle_environment:
description:
- Name of the lifecycle environment
type: str
content_view:
description:
- Name of the content view
type: str
subscriptions:
description:
- List of subscriptions that include either Name, Pool ID, or Upstream Pool ID.
- Pool IDs are preferred since Names and Upstream Pool IDs are not guaranteed to be unique. The module will fail if it finds more than one match.
type: list
elements: dict
suboptions:
name:
description:
- Name of the Subscription to be added.
- Mutually exclusive with I(pool_id) and I(upstream_pool_id).
type: str
required: false
pool_id:
description:
- Pool ID of the Subscription to be added.
- Mutually exclusive with I(name) and I(upstream_pool_id).
- Also named C(Candlepin Id) in the CSV export of the subscriptions,
- it is as well the C(UUID) as output by C(hammer subscription list).
type: str
required: false
upstream_pool_id:
description:
- Upstream Pool ID of the Subscription to be added.
- Mutually exclusive with I(name) and I(pool_id).
- Also named C(Master Pools) in the Red Hat Portal.
type: str
required: false
host_collections:
description:
- List of host collections to add to activation key
type: list
elements: str
content_overrides:
description:
- List of content overrides that include label and override state ('enabled', 'disabled' or 'default')
type: list
elements: dict
suboptions:
label:
description:
- Label of the content override
type: str
required: true
override:
description:
- Override value
choices:
- enabled
- disabled
- default
type: str
required: true
auto_attach:
description:
- Set Auto-Attach on or off
type: bool
release_version:
description:
- Set the content release version
type: str
service_level:
description:
- Set the service level
choices:
- Self-Support
- Standard
- Premium
type: str
max_hosts:
description:
- Maximum number of registered content hosts.
- Required if I(unlimited_hosts=false)
type: int
unlimited_hosts:
description:
- Can the activation key have unlimited hosts
type: bool
purpose_usage:
description:
- Sets the system purpose usage
type: str
purpose_role:
description:
- Sets the system purpose role
type: str
purpose_addons:
description:
- Sets the system purpose add-ons
type: list
elements: str
state:
description:
- State of the Activation Key
- If C(copied) the key will be copied to a new one with I(new_name) as the name and all other fields left untouched
- C(present_with_defaults) will ensure the entity exists, but won't update existing ones
default: present
choices:
- present
- present_with_defaults
- absent
- copied
type: str
new_name:
description:
- Name of the new activation key when state == copied
type: str
extends_documentation_fragment:
- theforeman.foreman.foreman
- theforeman.foreman.foreman.organization

validate_certs: "{{ foreman_validate_certs }}"
organization: "{{ rc_organization }}"
location: "{{ rc_location }}"
state: "{{ rc_state }}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is passing some state necessary for this module? What would it mean to ensure state for a registration command?

The documentation in the module has no state in the example:

EXAMPLES = '''
- name: "Generate registration command"
  theforeman.foreman.registration_command:
    username: "admin"
    password: "changeme"
    server_url: "https://foreman.example.com"
'''

Copy link
Contributor

@wbclark wbclark left a comment

Choose a reason for hiding this comment

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

Hey @stejskalleos could you also add a changelog fragment? Similar to f4ab021

def run(self, **kwargs):
new_entity = super(NestedParametersMixin, self).run(**kwargs)
if new_entity:
if new_entity and 'id' in new_entity:
Copy link
Member

Choose a reason for hiding this comment

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

Is this another hint, that it really should not be an entity module?

@stejskalleos stejskalleos changed the title Module for generating registration command [WIP] Module for generating registration command Sep 15, 2021
@evgeni
Copy link
Member

evgeni commented Sep 17, 2021

Hey @stejskalleos could you also add a changelog fragment? Similar to f4ab021

For new modules, no changelog entry is needed, it is autogenerated

@wbclark
Copy link
Contributor

wbclark commented Sep 17, 2021

For new modules, no changelog entry is needed, it is autogenerated

Interesting... thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants