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

Exporter configuration via Helm Chart doesn't sync crdsToDiscover and overwriteCrdsActions options #86

Open
irizzant opened this issue Nov 13, 2024 · 10 comments

Comments

@irizzant
Copy link

irizzant commented Nov 13, 2024

I have deployed Exporter with Helm chart version 0.2.36 in kubernetes 1.30 with the following helm chart configuration:

overwriteConfigurationOnRestart: true
eventListener:
  type: POLLING
configMap:
  name: config-k8s.yaml
  config:
    crdsToDiscover: ".metadata.ownerReferences[0].kind == \"CompositeResourceDefinition\" and .spec.scope == \"Namespaced\""
    overwriteCrdsActions: false
    resources:
    - kind: v1/namespaces
    .... # omitted for brevity

In Port this configuration translates into:

resources:
    - kind: v1/namespaces
    .... # omitted for brevity

The Kubernetes ConfigMap contains the right configuration, but it's not mirrored in Port though.

This is a problem because if I manually update the kubernetes datasource in Port adding the missing configurations:

crdsToDiscover: ".metadata.ownerReferences[0].kind == \"CompositeResourceDefinition\" and .spec.scope == \"Namespaced\""
    overwriteCrdsActions: false

then at the next restart of the exporter pod the configuration is lost.
On the other end, by setting overwriteConfigurationOnRestart: true I loose the possibility to keep the configuration in sync with Port

@abangser
Copy link

abangser commented Nov 22, 2024

Just adding a plus one and hopefully some helpful context. I have tried doing the same thing and I noticed in the exporter logs that I see:

crd.go:369] Discovering CRDs is disabled

I don't see any env vars or configs that can change this so I wonder if this may be the issue? But here is the config from my helms values file if that helps with the debugging:

configMap.config: |-
  crdsToDiscover: .spec.group == "mydomain.io"
  resources: []

Would love to fully automate my setup with Port and this would seem to do it 🤞

@omby8888
Copy link
Contributor

Hi @abangser, @irizzant ,
Thank you for reporting this issue! it has been resolved in version v0.4.5. Please update to the latest version and let us know if you encounter any further issues.

@abangser
Copy link

abangser commented Nov 25, 2024

Yay! Very exciting, thanks @omby8888 and team for the quick turn around 🎉 Do you have a timeline for a release to your helm chart? I see the code change in the chart, but don't think I see the released chart yet? 🤔

@omby8888
Copy link
Contributor

Hey @abangser, iv'e now also released the helm chart 😄

@irizzant
Copy link
Author

irizzant commented Dec 2, 2024

Hi @abangser thanks for the update, the overwriteCrdsActions is still not synced to the exporter configuration in Port but the crdsToDiscover part works fine. Can you check?

@abangser
Copy link

abangser commented Dec 8, 2024

Heyya @irizzant, so sorry for the delayed response.

I can confirm that I can use the Helm chart to set the crdsToDiscover via the configMap.config value.

Specifically, I used:

# port-config.yaml
resources: []
crdsToDiscover: >-
  "mykind.myorg.io" as $crdNames | .metadata.name |  test($crdNames)

# Helm command
helm upgrade --install port-exporter port-labs/port-k8s-exporter \
  --version 0.2.37 \
  --namespace port-k8s-exporter \
  --create-namespace \
  --set stateKey=my-port-exporter \
  --set createDefaultResources=false \
  --set overwriteConfigurationOnRestart=true \
  --set secret.useExistingSecret=true \
  --set-file configMap.config=port-config.yaml

And with that, I saw the blueprint, integration, and self-serve actions all populate which was great 🎉

I did however get surprised that any updates to the CM didn't seem to reflect in the UI. Once this has synced for the first time, is it expected to then update the config settings via the UI or the API? For completeness, what I saw was:

  • When I update in the ConfigMap, nothing changes in how the exporter behaves or the settings visible in the UI
  • When I update via the UI, the exported uses the new setting, but the CM doesn't change

Therefore, it seems like the CM is being used as a one off setup? Is that the right understanding? This seems reinforced by the docs here.

If my understanding is correct, I am a bit worried about config being a required field, but being also so likely to be out of date with the working value in the UI/API. I would maybe then want to just set up the integration via the API/UI and install the exporter deployment without the comprehensive details in the helm chart. But that doesn't seem to be a provided option at the moment 💭

@irizzant
Copy link
Author

irizzant commented Dec 9, 2024

This is strange, I don't seem to have this behaviour.

@abangser
Copy link

abangser commented Dec 9, 2024

I wouldn't be surprised if it is noob behaviour. @irizzant I have shot you over a video in the community slack in case that helps pin down the issue. And will of course update here when we figure it out!

@shalev007
Copy link

shalev007 commented Dec 10, 2024

@irizzant We'll take a look at the overwriteCrdsActions issue and update 👀
@abangser Thanks for the response! 🙏
But in your message there are some things I don't understand, the CM is basically for GitOps configuration management,
I think the default installation command do not include the CM at all. if you would like to add the crdsToDiscover you should be able to do that and still control the mapping via the UI.

@abangser
Copy link

Thanks so much @shalev007 for the update. Yes, I know I can do it via the UI, but I'm hoping to do it all via declarative code with the helm chart.

My assumption is that a gitops install method would continue to sync against the declared values. E.g. if the CM used by the port exporter pod updates, then the exporter pod would use that new config (also displaying that in the UI). That assumption seems to not be right though based on my experiences (I can send you the video on slack as well!) and I'm trying to understand if it is by design or something I should raise as another improvement 🤔

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

No branches or pull requests

4 participants