Skip to content

Conversation

antoniomacri
Copy link

@antoniomacri antoniomacri commented Apr 15, 2025

This PR sorts by name only the env vars generate in the VanillaKubernetesProcessor (and DevClusterHelper, I don't know if this is needed as well). However, other vars generated from other parts of the code are not taken into account, for instance the KUBERNETES_NAMESPACE var.

For me this is mostly sufficient, since the namespace var does not change and is always the first one.

However:

  1. how can provide a more general solution which sorts all env vars? Do I need to hook into dekorate?
  2. there is no test. Can you give me some hint on where to put them if they are needed?

@geoand geoand requested a review from iocanel April 15, 2025 09:30

This comment has been minimized.

Copy link
Contributor

@iocanel iocanel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks awesome and easier than I initally thought.

Can we please add an integration test in integration-tests/kubernetes/quarkus-standard-way/ to ensure we won't run into regressions in the future?

@antoniomacri
Copy link
Author

This looks awesome and easier than I initally thought.

Can we please add an integration test in integration-tests/kubernetes/quarkus-standard-way/ to ensure we won't run into regressions in the future?

Thanks! Yes, I will add an integration test.

@antoniomacri
Copy link
Author

I've added an integration test.

Do you think it is preferrable to sort the env vars only if idempotent=true (as I did) or better to do it always?

@antoniomacri
Copy link
Author

@iocanel just making sure you did not miss my commit and comment above

@vivynz
Copy link

vivynz commented May 8, 2025

Hi, do you have any updates on this PR? It would help simplify our pipeline process.

@gastaldi gastaldi requested a review from iocanel May 14, 2025 14:18
@gastaldi
Copy link
Contributor

Can you please squash the commits and force-push? Thanks

This comment has been minimized.

@antoniomacri
Copy link
Author

@gastaldi @iocanel any news on this?

Copy link

quarkus-bot bot commented May 23, 2025

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 568f733.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

Copy link
Contributor

@iocanel iocanel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGMT

@antoniomacri
Copy link
Author

Hi @gastaldi, when do you plan to merge and release it?

@gastaldi gastaldi merged commit 43f2aa9 into quarkusio:main Jun 16, 2025
27 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.25 - main milestone Jun 16, 2025
@gastaldi
Copy link
Contributor

Thanks!

@quarkus-bot quarkus-bot bot added the kind/enhancement New feature or request label Jun 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Prevent reordering of env in kubernetes deployment

4 participants