CLOUDP-66908: add configmap to evg logs

[bznein]

All Submissions:

• Have you signed our CLA?
• Have you checked to ensure there aren't other open Pull Requests for the same update/change?

[chatton]

Looks great! Nice and straight forward change.

Good to merge once tests pass 👍 (and we see the configmaps are correctly attached!)

[rodrigovalin]

Check that ApiException capture, return inmediatelly maybe?

[rodrigovalin]

if the ApiException is raised, config_maps will be None and .items will fail with AttributeError

[bznein]

Ah you are absolutely right! I'll fix this on all functions in this file!

[chatton]

❗ nice catch

[bznein]

Should we change the function to return a Optional[dict] and similar and give the caller the burden to check it?
The other way would be to just not catch the exception, but that would interrupting the dumping of logs and mark the task as failed even when it is not. So I'm leaning towards the first.

[chatton]

@bznein a few small things

1. Since we are attaching all configmaps, this means we end up attaching kube-configs. (definitely don't want this)
2. The format of automation config in the configmap is not very nice to look at! In this case, maybe we just want to pretty-print the .data.automation-config value, and not get the output of the configmap resource itself.

What do you think?

[bznein]

@bznein a few small things

1. Since we are attaching all configmaps, this means we end up attaching kube-configs. (definitely don't want this)
2. The format of automation config in the configmap is not very nice to look at! In this case, maybe we just want to pretty-print the .data.automation-config value, and not get the output of the configmap resource itself.

What do you think?

1. You are right. I didn't want to have a function too specific that looks for a particular configmap in case the name changes in the future. Should we do that instead? Or exclude kube-config?
2. Yes I agree, I'll see which way is better

[chatton]

@bznein a few small things

1. Since we are attaching all configmaps, this means we end up attaching kube-configs. (definitely don't want this)
2. The format of automation config in the configmap is not very nice to look at! In this case, maybe we just want to pretty-print the .data.automation-config value, and not get the output of the configmap resource itself.

What do you think?

1. You are right. I didn't want to have a function too specific that looks for a particular configmap in case the name changes in the future. Should we do that instead? Or exclude kube-config?
2. Yes I agree, I'll see which way is better

I think in this case, it might actually be fine to just pick a configmap by name, and explicitly just upload that one. Right now, we only have a single one we want, later on we can worry about adding all, or maybe use an exlude-list like you mentioned.

We could maybe create a mechanism like dump_configmap_value(cm, key="automation-config", pretty=True)

I don't think we need to worry too much about configmap names changing, as it should always be a deterministic name based on the name of the resource. "{mdb-name}-config" I think.

Another thing to consider, we are currently running all our tests in kind, a pretty empty environment, but if we're running in a different cluster, there can be all sorts of other k8s resources we don't actually care about, so maybe explicitly stating which resources we want will result in a cleaner output overall.