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

Regression since 2.6.3.Final in OpenShift extension #23417

Closed
edeandrea opened this issue Feb 3, 2022 · 15 comments · Fixed by #23239
Closed

Regression since 2.6.3.Final in OpenShift extension #23417

edeandrea opened this issue Feb 3, 2022 · 15 comments · Fixed by #23239
Assignees
Labels
area/kubernetes kind/bug Something isn't working
Milestone

Comments

@edeandrea
Copy link
Contributor

edeandrea commented Feb 3, 2022

Describe the bug

Seems when the openshift extension generates the resources its no longer generating the ImageStream resource properly.

See https://github.com/quarkusio/quarkus-super-heroes/blob/5a805fadaeff91572345777775a943d5324c67be/rest-fights/deploy/k8s/native-java17-openshift.yml#L83-L91 (generated with the openshift extension on Quarkus 2.7.0.Final) vs https://github.com/quarkusio/quarkus-super-heroes/blob/c2e744ec28defd9d97b4ba578d1e0f16eeb3ea8d/rest-fights/deploy/k8s/native-java17-openshift.yml#L83-L103 (generated with the openshift extension on Quarkus 2.6.3.Final).

Expected behavior

The generated ImageStream resource doesn't have a metadata.name value. Even still as you can see from the 2 links

Actual behavior

No response

How to Reproduce?

No response

Output of uname -a or ver

No response

Output of java -version

No response

GraalVM version (if different from Java)

No response

Quarkus version or git rev

No response

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

No response

@edeandrea edeandrea added the kind/bug Something isn't working label Feb 3, 2022
@quarkus-bot
Copy link

quarkus-bot bot commented Feb 3, 2022

/cc @geoand, @iocanel

@geoand
Copy link
Contributor

geoand commented Feb 3, 2022

@iocanel has this already been fixed?

edeandrea added a commit to quarkusio/quarkus-super-heroes that referenced this issue Feb 3, 2022
Rollback to Quarkus 2.6.3 due to quarkusio/quarkus#23417
@iocanel
Copy link
Contributor

iocanel commented Feb 3, 2022

@iocanel has this already been fixed?

What we fixed was adding the missing ImageStream. In this particular case, the ImageStream is there but is missing the metadata.name.

@iocanel iocanel self-assigned this Feb 3, 2022
@edeandrea
Copy link
Contributor Author

edeandrea commented Feb 3, 2022

It doesn't seem though like the labels on it are correct. The name: fights-db under metadata.labels isn't correct either (both in the 2.6.3 & 2.7.0 version). I'm not sure where it is pulling that label from? Shouldn't it be name: rest-fights?

@iocanel
Copy link
Contributor

iocanel commented Feb 3, 2022

The missing metadata.name should be fixed by: ffc5957#diff-2f7f8b1e6f4a44f6f156a44d9a60ec9f4a233e7874e8059fb0c627771a505fc4R271

@iocanel
Copy link
Contributor

iocanel commented Feb 3, 2022

It doesn't seem though like the labels on it are correct. The name: fights-db under metadata.labels isn't correct either (both in the 2.6.3 & 2.7.0 version). I'm not sure where it is pulling that label from? Shouldn't it be name: rest-fights?

I can't tell about the labels, unless I see the configuration. Let me have a look at your project....

@edeandrea
Copy link
Contributor Author

I can't tell about the labels, unless I see the configuration. Let me have a look at your project....

The application.properties file is at https://github.com/quarkusio/quarkus-super-heroes/blob/main/rest-fights/src/main/resources/application.properties and the command used to generate would be

rest-fights/mvnw -f rest-fights/pom.xml versions:set clean package -DskipTests -DnewVersion=native-java17-latest -Dquarkus.container-image.tag=native-java17-latest -Dquarkus.kubernetes.deployment-target=openshift -Dquarkus.kubernetes.version=native-java17-latest -Dquarkus.kubernetes.ingress.expose=true -Dquarkus.kubernetes.resources.limits.memory=128Mi -Dquarkus.kubernetes.resources.requests.memory=32Mi '-Dquarkus.kubernetes.annotations."app.quarkus.io/vcs-url"=https://github.com/quarkusio/quarkus-super-heroes' '-Dquarkus.kubernetes.annotations."app.quarkus.io/vcs-ref"=main' -Dquarkus.openshift.version=native-java17-latest -Dquarkus.openshift.route.expose=true -Dquarkus.openshift.resources.limits.memory=128Mi -Dquarkus.openshift.resources.requests.memory=32Mi '-Dquarkus.openshift.annotations."app.openshift.io/vcs-url"=https://github.com/quarkusio/quarkus-super-heroes' '-Dquarkus.openshift.annotations."app.openshift.io/vcs-ref"=main' -Dquarkus.knative.version=native-java17-latest '-Dquarkus.knative.labels."app.openshift.io/runtime"=quarkus' -Dquarkus.knative.resources.limits.memory=128Mi -Dquarkus.knative.resources.requests.memory=32Mi '-Dquarkus.knative.annotations."app.openshift.io/vcs-url"=https://github.com/quarkusio/quarkus-super-heroes' '-Dquarkus.knative.annotations."app.openshift.io/vcs-ref"=main'

https://github.com/quarkusio/quarkus-super-heroes/runs/5054787642?check_suite_focus=true#step:4:11282

@iocanel
Copy link
Contributor

iocanel commented Feb 3, 2022

The labels thing must be a bug triggered by the fact that there are multiple Deployment resources in the final manifest. This is something that we should do a better job at handling. I'll work on it ASAP.

The missing metadata.name should be fixed in the main branch already. Let me checkout your project to verify.

@edeandrea
Copy link
Contributor Author

edeandrea commented Feb 3, 2022

The labels thing must be a bug triggered by the fact that there are multiple Deployment resources in the final manifest. This is something that we should do a better job at handling. I'll work on it ASAP.

Yeah if in src/main/kubernetes/openshift.yml it seems that whatever labels/annotations are on the last Deployment in there are carried forward. They shouldn't be.

I would think they should only be carried forward if the Deployment.metadata.name (or DeploymentConfig.metadata.name) matches the metadata.name of any of the generated resources.

@iocanel
Copy link
Contributor

iocanel commented Feb 3, 2022

The missing metadata.name is already fixed on master. And the labels are indeed broken, as it seems to pickup the labels from your postgres deployment, instead of the app.

@edeandrea
Copy link
Contributor Author

The missing metadata.name is already fixed on master.

Awesome! Will that make it into 2.7.1?

@gsmet
Copy link
Member

gsmet commented Feb 3, 2022

@iocanel what is the PR fixing this metadata.name issue?

@iocanel
Copy link
Contributor

iocanel commented Feb 3, 2022

@gsmet: It's #23239

@gsmet
Copy link
Member

gsmet commented Feb 3, 2022

OK, so I can confirm this particular fix is planned for 2.7.1.Final.

@edeandrea
Copy link
Contributor Author

And the labels are indeed broken, as it seems to pickup the labels from your postgres deployment, instead of the app.

Should I open a new issue @iocanel for this?

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.

4 participants