Adding general map field for configuring AEE #1102
Conversation
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jpodivin The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
jpodivin
force-pushed
the
aeeconfig
branch
2 times, most recently
from
6ad24e6
to
ad8e3a0
Compare
jpodivin
force-pushed
the
aeeconfig
branch
2 times, most recently
from
76bb213
to
6243d58
Compare
| } | ||
|
|
||
| // Check presence of the key, type and contents | ||
| configMapName, test := cm.Data["envConfigMapName"] |
There was a problem hiding this comment.
is it that the CM named d.Deployment.Spec.AnsibleEEConfig stores the name of another CM that actually stores the config? if so, why is that the case?
There was a problem hiding this comment.
Idea is that the AnsibleEEConfig will be catch all place for all the configuration options that don't necessarily need their own field. Whether they will be defined as config maps, such as in this case, or plain values.
In this case we are storing name of a config map, which is used for environment variables. So yes, we could just remove that one level and get the same value. But it would prevent us from using this field for anything else. At some point in the future, if customer came along and asked, we would have to add another field, and another. This way we just put it in a general config map, and let this function do whatever is necessary with it.
Signed-off-by: Jiri Podivin <[email protected]>
|
@jpodivin: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
I like the overall idea here, but I would like to see this proposed as a feature in Jira, aligned against the next FR (FR2), so we can properly scope and plan with QE and docs. |
New config map holding data for additional AEE configuration. By it's nature it's open ended and all the fields will require checking. Normally we would could use verification in schema for this. But since we don't want to change signature of resources too much conditionals are a way to go.