-
Notifications
You must be signed in to change notification settings - Fork 6
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
molecule converge -> test #195
Conversation
@@ -169,6 +169,7 @@ | |||
old_root_content != tas_single_node_fulcio.root_ca | |||
and (tas_single_node_fulcio.private_key != "" | |||
or (certs_dir_files.files | selectattr('path', 'equalto', tas_single_node_remote_fulcio_auto_generated_private_key) | list | length) == 0) | |||
changed_when: false |
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 changed_when
shouldn't really be necessary IMO. Assuming the when
logic works fine, this task will only trigger if a certain config change happened, but rerunning again after that shouldn't trigger it again, should 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.
Correct - I checked again after this comment and realized the slurped content was not being decoded hence it being wrong on every single run. Fulcio should now be all fixed (I pray)
@@ -166,10 +166,9 @@ | |||
remote_src: true | |||
mode: '0600' | |||
when: > | |||
old_root_content != tas_single_node_fulcio.root_ca | |||
old_root_content.content | b64decode != tas_single_node_fulcio.root_ca |
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.
@SequeI
could you please do the same for
or old_root_content != tas_single_node_fulcio.root_ca) |
(better to have this change in the same PR)
Thanks!
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.
LGTM!
Thanks!
Changed converge -> test
Fixed one task to pass idempotence checks
Should be all good now, esp for finding more issues and bugs