-
Notifications
You must be signed in to change notification settings - Fork 145
Fix for issue #115 #116
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
Fix for issue #115 #116
Conversation
fabianvf
left a comment
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'm nervous about merging a change like this, because it will change the valid argument list for every single resource in openshift-ansible-kubernetes, and likely break everybody who has playbooks relying on them. I think a smaller change that just avoids creating conflicts is probably a better approach to take.
| if prop_alt_prefix != prop_prefix: | ||
| if prop_alt_prefix: | ||
| args[prop_prefix + prop_name]['aliases'] = [prop_alt_prefix + prop_name] | ||
| elif prop_prefix: |
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 think rather than removing these branches entirely, it might be better to just add a check that ensures it's not creating an alias that conflicts with a top level key.
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.
That may even just entail keeping a set of all names + aliases that have been used and checking that everything we're setting is unique.
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.
Yes, this make sense. Will consider this option too
|
The generated modules are deprecated in favor of the new dynamic client and corresponding k8s module |
Original code adds aliases to arf_spec, which often cause conflicts with top-level properties
closes #115