Skip to content
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

How can related labels be defined as code for JT/WFJT/WFNodes #496

Closed
adonisgarciac opened this issue Feb 10, 2023 · 5 comments · Fixed by #500
Closed

How can related labels be defined as code for JT/WFJT/WFNodes #496

adonisgarciac opened this issue Feb 10, 2023 · 5 comments · Fixed by #500

Comments

@adonisgarciac
Copy link
Contributor

https://github.com/redhat-cop/controller_configuration/blob/devel/roles/workflow_job_templates/tasks/add_workflows_schema.yml#L23

Why we are allowing something like __workflow_loop_node_item.related.labels? How can a JT/WF/WFNode object be defined as code with related labels?

It is getting an error because it has default([]) so it will always send a value even it is not defined and for versions previous of AAP 2.3, labes are not in workflow_job_template_node.

I would change these lines:

Before:

    labels:                               "{{ __controller_template_item.labels | default(__controller_template_item.related.labels | default([]) | map(attribute='name') | list) | default(omit) }}"

After:

    labels:                               "{{ __controller_template_item.labels | default(omit) }}"
@sean-m-sullivan
Copy link
Collaborator

This is due to export output being interpreted from the awx cli. It is done as a full object with org and other info included
See this output

While we started with what I call Simple Definition where its just a list, we also adopted export output must be allowed for those using that. The output is defaulted this way for Labels, Inventories, credentials, notifications, and a TON of other objects as that is how they work in the API and the way export looks them up.

@adonisgarciac
Copy link
Contributor Author

Perfect, understood! Thank you for the explanation @sean-m-sullivan

Then, we will have to see how to address this problem because, currently, it will break if it is using some target platform different than AAP 2.3 because labels in workflow_nodes are new in that version.

Since I changed default(omit, true) to default(omit) in order to allow to empty labels of JT/WFJT, a default value of empty list is sending if anything is defined. So it will break for workflow_nodes.

I will check it out and I will try to do a PR fixing it and keeping every scenario.

@sean-m-sullivan
Copy link
Collaborator

That I think is why we were doing that in the first place, to not send an item if none is defined, its a catch 42 for sure in terms of config as code, when I tested it, it didn't wipe out labels I had set if I didn't set it. and maybe we need to change it on the module level? which is something we CAN propose, If you can post here some scenarios of what you are providing and the behavior we are seeing, I think it will help people and me understand the issue, so we can see what you WANT to achieve.

@djdanielsson
Copy link
Collaborator

djdanielsson commented Feb 11, 2023

I think the correct answer is omit, if you want to force something to be blank you should have to pass "". We can put this as a talking point for the community meeting.

@adonisgarciac
Copy link
Contributor Author

The problem itself is not to have default(omit). I mean, if you have default(omit) and you have not configured that parameter in your object as code, anything will be sent.

Actually, the problem for labels is that we have default([]) and then, default(omit) will not apply anyway and always it will be sent a value (empty list in case that labels parameter is not present in the config as code).

If you can post here some scenarios of what you are providing and the behavior we are seeing

Behavior before to #491

It was impossible to clean the labels of an object if some label have been configured previously. (e.g), a JT with a label is created. Afterwards, you want to remove that label, you will not be able to do it from config as code because default(omit,true) will omit an empty defined label in the config as code.

After to #491

In order to clean the labels of an object, you can achieve it defining an empty labels parameter in your object defined as code. It would be perfect,however, for labels, we have a previous default value configured:

"{{ __controller_template_item.labels | default(__controller_template_item.related.labels | default([]) | map(attribute='name') | list) | default(omit) }}"

Then, even though you haven't configured labels for the object, ALWAYS an empty label will be sent. What is the problem now? the problem is with workflow_job_template_node which include labels as new parameter for AAP2.3 so, it will fail for the rest of versions.

In case that we change default(omit,true) for default(omit), it will not bring problems unless some another default value has been defined like labels parameter has.

adonisgarciac added a commit to automationiberia/infra.aap_configuration that referenced this issue Feb 14, 2023
przemkalit pushed a commit to przemkalit/aap_configuration that referenced this issue Nov 22, 2024
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 a pull request may close this issue.

3 participants