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

Support hot reloading for mounted volume configmaps and secrets #280

Open
StephanePaulus opened this issue Mar 16, 2021 · 9 comments
Open
Labels
info: good first issue Good for newcomers type: enhancement New feature or request

Comments

@StephanePaulus
Copy link

The documentation says that this library supports reading configmaps from mounted volumes. I have not been able to find a way to make it work nor have I seen the code that would make this possible.

Also the library only supports watching for changes of configmaps.

I modified the code to support reading both configmaps and secrets from mounted volumes and also watch for changes to both of them. code

I am not feeling comfortable enough to already make a pull request. The code has lots of replication in it and I have some issues with writing/running tests.

I would like to ask for advice and if there is a need for this besides my own. In my case I am not allowed to use the kubernetes API, so mounted volumes is the only way I can read configmaps or secrets.

@pgressa
Copy link
Contributor

pgressa commented Mar 18, 2021

Hi @StephanePaulus, first of all huge thank you for taking this initiative and POC!

library supports reading configmaps from mounted volumes.

Yeah, this is definitely smth we do not support yet, I'll fix the guideline, thanks for this and sorry for misleading you.

I would like to ask for advice and if there is a need for this besides my own. In my case I am not allowed to use the kubernetes API, so mounted volumes is the only way I can read configmaps or secrets.

That's good point and use case. Feel free to create PR and I can take it over if you like or I can provide you initial code review and you can finish the work yourselves.

Thanks again.

@pgressa pgressa added type: enhancement New feature or request info: good first issue Good for newcomers labels Mar 18, 2021
@StephanePaulus
Copy link
Author

Hi @pgressa, please let me know if you feel like the PR is not what you were expecting. Maybe we can come up with a better solution. If you are waiting for me to fully complete the PR, with all test passing please let me know.

@pgressa
Copy link
Contributor

pgressa commented Jun 11, 2021

Hi @StephanePaulus, first of sorry for being quite for some time. Once again thanks for the PR. I'd like to get beck to this feature now.

To answer question #315:
For the secrets case I think we could either add the file path of the variables, so for the secret :

apiVersion: v1
kind: Secret
metadata:
  name: mysecret
type: Opaque
data:
  username: YWRtaW4=
  password: MWYyZDFlMmU2N2Rm

and configuration

kubernetes:
  client:
    enabled: false
    watch: true
    secrets:
      enabled: true
      paths:
        - /etc/example-service/secrets

So the property source name would be then:

  • file path etc.example-servce.secrets.username resp etc.example-servce.secrets.password

Another option is to extend the configuration with the defined prefix:

kubernetes:
  client:
    enabled: false
    watch: true
    secrets:
      enabled: true
      items:
        - path: /etc/example-service/secrets
           namePrefix: foo.bar 

What do you think?

Where to place it
Ideally this feature can find its way into https://github.com/micronaut-projects/micronaut-discovery-client in the future but at this moment let's keep it focused entirely on K8s use case.

I'll take over the PR and continue on it if you're not against it.

@StephanePaulus
Copy link
Author

@pgressa yes sure you can take over. I could share the new code I have in a private repository, that already fixes some bad file watching stuff. Let me know if I can be of any assistance.

I think the namePrefix option is the cleanest.

@pgressa
Copy link
Contributor

pgressa commented Jun 14, 2021

I was thinking about deduplicating the code by providing more generic approach like:

kubernetes:
  client:
    external:
      - name: <optional-name-of-the-property>, defaults to the "basename <path>"
         path: <mandatory-path-to-file>
         type: <mandatory-file-type> [file-per-property|structered-configuration], no default
         watch: [true/false]
         enabled: [true/false]
  • This approach brings certain level of extensibility by providing the type file resolver.

also @StephanePaulus if you could share the new code I would be glad for it. Thanks.

@StephanePaulus
Copy link
Author

@pgressa here you can find the updated code that worked good enough for me. It is not perfect, but it did the job. code repo

Your approach looks very generic and will solve all issues I am trying to fix.

How kubernetes adds mounted files to the filesystem of a container is kind of tricky. When using the normal filewatcher you need to do special logic, because the files are just symbolic links. And the main folder is "..data"

@StephanePaulus
Copy link
Author

@pgressa Sorry to ask you, but do you have an idea when this feature/fix can be available?

@pgressa
Copy link
Contributor

pgressa commented Sep 21, 2021

@StephanePaulus sorry for the delay, I have extracted the loading of config maps mounted as volumes into this PR #365. The hot reloading will be subject of some other PR in the future.

@pgressa pgressa changed the title Add support to load configmaps when mounted as a volume. Support hot reloading for mounted volume configmaps and secrets Support hot reloading for mounted volume configmaps and secrets Sep 21, 2021
@pgressa
Copy link
Contributor

pgressa commented Sep 21, 2021

@StephanePaulus I have removed the part "Add support to load configmaps when mounted as a volume. " from the title, since that's is already merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
info: good first issue Good for newcomers type: enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants