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

Kubernetes env vars generation changed unexpectedly #9629

Closed
Ladicek opened this issue May 27, 2020 · 14 comments · Fixed by #9758
Closed

Kubernetes env vars generation changed unexpectedly #9629

Ladicek opened this issue May 27, 2020 · 14 comments · Fixed by #9758
Assignees
Labels
area/kubernetes kind/bug Something isn't working
Milestone

Comments

@Ladicek
Copy link
Contributor

Ladicek commented May 27, 2020

Describe the bug
I have these lines in my application.properties:

quarkus.openshift.env-vars.DB_DATABASE.secret=postgresql
quarkus.openshift.env-vars.DB_DATABASE.value=database

And I expect Quarkus Kubernetes to generate an env var that corresponds to the database field of the postgresql secret. This is how it works in Quarkus 1.3.3, at least:

        - name: "DB_DATABASE"
          valueFrom:
            secretKeyRef:
              key: "database"
              name: "postgresql"
              optional: false

However, when trying on Quarkus 1.5.0.CR1, I only get this:

        - name: "DB_DATABASE"
          value: "database"

Expected behavior
See above.

Actual behavior
See above.

To Reproduce
Steps to reproduce the behavior:

  1. Add quarkus-openshift.
  2. Add configuration per above to application.properties.
  3. mvn clean package
  4. See target/kubernetes/openshift.yml

Configuration
See above.

Environment (please complete the following information):

  • Output of uname -a or ver:
    lthon@argondie:~$ uname -a
    Linux argondie 4.15.0-101-generic #102-Ubuntu SMP Mon May 11 10:07:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
    
  • Output of java -version:
    lthon@argondie:~$ java -version
    openjdk version "1.8.0_252"
    OpenJDK Runtime Environment (build 1.8.0_252-8u252-b09-1~18.04-b09)
    OpenJDK 64-Bit Server VM (build 25.252-b09, mixed mode)
    
  • GraalVM version (if different from Java): N/A
  • Quarkus version or git rev: 1.5.0.CR1
  • Build tool (ie. output of mvnw --version or gradlew --version):
    lthon@argondie:~$ mvn --version
    Apache Maven 3.6.3 (cecedd343002696d0abb50b32b541b8a6ba2883f)
    Maven home: /home/lthon/software/apache-maven
    Java version: 1.8.0_252, vendor: Private Build, runtime: /usr/lib/jvm/java-8-openjdk-amd64/jre
    Default locale: cs_CZ, platform encoding: UTF-8
    OS name: "linux", version: "4.15.0-101-generic", arch: "amd64", family: "unix"
    

Additional context
See also https://quarkusio.zulipchat.com/#narrow/stream/187038-dev/topic/Kubernetes.20env.20vars

@Ladicek Ladicek added kind/bug Something isn't working area/kubernetes labels May 27, 2020
@Ladicek
Copy link
Contributor Author

Ladicek commented May 27, 2020

CC @geoand @iocanel

@geoand
Copy link
Contributor

geoand commented May 27, 2020

Also cc @metacosm

@metacosm
Copy link
Contributor

I will take a look ASAP.

@iocanel
Copy link
Contributor

iocanel commented May 27, 2020

CC @geoand @iocanel

Hmmm, I didn't realize this used to work. In fact I was about ti implement it now.

@metacosm
Copy link
Contributor

metacosm commented May 27, 2020 via email

@Ladicek
Copy link
Contributor Author

Ladicek commented May 27, 2020

I'm fine with the old syntax not working, if there's a replacement and migration documentation.

In fact, just today I've learnt (in the linked Zulip topic) that there's a new syntax for env vars coming in 1.5 (a82951f), but there's nothing in the 1.5 migration guide.

@iocanel
Copy link
Contributor

iocanel commented Jun 1, 2020

In the new syntax we need to implement support for something like:

quarkus.kubernetes.secret.SECRET_NAME.secret-key=SECRET_KEY
quarkus.kubernetes.secret.SECRET_NAME.name=ENV_VAR_NAME

AFAIR, @metacosm expressed the desire to work on this issue, so I'll assign it to him.

@Ladicek
Copy link
Contributor Author

Ladicek commented Jun 1, 2020

I will repeat that until someone acks -- we need the migration guide content!

@metacosm
Copy link
Contributor

metacosm commented Jun 2, 2020

I will repeat that until someone acks -- we need the migration guide content!

That said, the old syntax should still be working for the time being.

@Ladicek
Copy link
Contributor Author

Ladicek commented Jun 2, 2020

I think I have clearly demonstrated that some of it does not :-)

@metacosm
Copy link
Contributor

metacosm commented Jun 2, 2020

I think I have clearly demonstrated that some of it does not :-)

Yep, but the intent is to remedy to this situation :)

metacosm added a commit to metacosm/quarkus that referenced this issue Jun 3, 2020
This is needed to be able to address quarkusio#9629. Things are still not
completely working but getting there.
@metacosm
Copy link
Contributor

metacosm commented Jun 4, 2020

I've completed the first pass on a fix in the #9758 PR and added support for defining vars from secret/configmap keys using the following syntax:

quarkus.kubernetes.env.vars-from-secret.foo.key=keyName
quarkus.kubernetes.env.vars-from-secret.foo.secret=secretName

which would create the following:

env:
  - name: FOO
     valueFrom: 
        secretKeyRef:
          key: keyName
          name: secretName
         optional: false

and would be equivalent to the previous syntax:

quarkus.kubernetes.env-vars.foo.secret=secretName
quarkus.kubernetes.env-vars.foo.value=keyName

Let me know what you think, @Ladicek @iocanel, please.

@Ladicek
Copy link
Contributor Author

Ladicek commented Jun 4, 2020

That looks good to me.

metacosm added a commit to metacosm/quarkus that referenced this issue Jun 5, 2020
metacosm added a commit to metacosm/quarkus that referenced this issue Jun 7, 2020
metacosm added a commit to metacosm/quarkus that referenced this issue Jun 9, 2020
metacosm added a commit to metacosm/quarkus that referenced this issue Jun 11, 2020
@metacosm
Copy link
Contributor

For reference, the new syntax is:

quarkus.kubernetes.env.mapping.foo.with-key=keyName
quarkus.kubernetes.env.mapping.foo.from-secret=secretName

since that was changed during the PR review process.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/kubernetes kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants