[pick-changes] | Merge pull request #1970 from confluentinc/Java-21-A… #1997
Conversation
…NSIENG-4487 Java 21 ansieng 4487
There was a problem hiding this comment.
Pull Request Overview
This PR migrates configurations to support Java 21 and adjusts several molecule test playbooks accordingly. Key changes include updating Java version assertions and custom Java paths; commenting out multiple platform and test blocks in molecule files to likely reflect deprecated or temporarily disabled configurations; and modifying an error message in a verification playbook.
Reviewed Changes
Copilot reviewed 37 out of 44 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| molecule/mini-setup-oauth-mtls/molecule.yml | Minor comment formatting update in cluster group comment |
| molecule/kerberos-rhel/verify.yml | Updated expected error message string for connector verification |
| molecule/custom-user-plaintext-rhel/molecule.yml | Comment adjustments to avoid port collision |
| molecule/connect-scale-up/verify.yml | Complete commenting-out of SSL property test block |
| molecule/confluent-kafka-kerberos-customcerts-rhel/molecule.yml | Disabled control-center1 platform configuration via comments |
| molecule/certificates.yml | Certificate hostname writing task commented out |
| molecule/ccloud/verify.yml | Multiple property tests have been disabled via comments |
| molecule/ccloud/prepare.yml | Disabled control_center inventory entry |
| molecule/broker-scale-up/converge.yml | Disabled broker verification tasks in control-center properties |
| molecule/archive-zookeeper-tls-rhel-fips/molecule.yml | Disabled control-center1 platform configuration via comments |
| molecule/archive-plain-debian/verify.yml | Java version assertion updated from 17 to 21 |
| molecule/archive-plain-debian/molecule.yml | Custom Java path updated from /opt/jdk17 to /opt/jdk21 |
Files not reviewed (7)
- molecule/Dockerfile-amazonlinux2023.j2: Language not supported
- molecule/Dockerfile-centos8-base.j2: Language not supported
- molecule/Dockerfile-debian-java21.j2: Language not supported
- molecule/Dockerfile-debian12.j2: Language not supported
- molecule/Dockerfile-rhel-java21.j2: Language not supported
- molecule/Dockerfile-ubuntu-java21.j2: Language not supported
- molecule/Dockerfile-ubuntu-java8.j2: Language not supported
Comments suppressed due to low confidence (3)
molecule/connect-scale-up/verify.yml:198
- If disabling this test block is intentional, please add a clarifying comment explaining the rationale to ensure clarity around test coverage adjustments.
# - name: Check line connect cluster ssl
molecule/ccloud/verify.yml:152
- Provide a rationale for disabling these property verification tests; clarifying whether they are deprecated or temporarily disabled will help maintain proper test coverage.
# - name: Check Max Message Bytes Property
molecule/broker-scale-up/converge.yml:59
- Commenting out these verification tasks can affect test coverage; add a comment explaining whether these tasks are deprecated or being replaced to ensure clarity.
# - name: Verify New brokers are not there in control-center properties
| @@ -237,7 +237,7 @@ | |||
| vars: | |||
| connect_url_input: "{{ kafka_connect_url }}" | |||
| active_connectors_input: "{{ add_non_existent_connector }}" | |||
| expected_message: "Connectors added or updated: test-connector-1: no configuration change, test-connector-2: ERROR error while adding new connector configuration (HTTP Error 500: Internal Server Error)." | |||
| expected_message: "Connectors added or updated: test-connector-1: no configuration change, test-connector-2: ERROR error while adding new connector configuration (HTTP Error 500: Server Error)." | |||
There was a problem hiding this comment.
Consider reviewing the updated error message; using 'Server Error' instead of the standard 'Internal Server Error' may lead to mismatches if the application returns the expected standard phrase.
| expected_message: "Connectors added or updated: test-connector-1: no configuration change, test-connector-2: ERROR error while adding new connector configuration (HTTP Error 500: Server Error)." | |
| expected_message: "Connectors added or updated: test-connector-1: no configuration change, test-connector-2: ERROR error while adding new connector configuration (HTTP Error 500: Internal Server Error)." |
Copilot uses AI. Check for mistakes.
| @@ -149,21 +149,21 @@ platforms: | |||
| privileged: true | |||
| networks: | |||
| - name: confluent | |||
There was a problem hiding this comment.
Add a comment detailing the reason for disabling the 'control-center1' platform configuration to assist future maintainers in understanding the change.
| - name: confluent | |
| - name: confluent | |
| # The 'control-center1' platform configuration has been disabled due to compatibility issues with the current setup. | |
| # Specifically, it requires additional resources and configurations that are not supported in this environment. | |
| # Future maintainers can re-enable this configuration after addressing the compatibility concerns. |
Copilot uses AI. Check for mistakes.
| @@ -149,12 +149,12 @@ | |||
| loop: "{{groups['ksql']}}" | |||
| when: groups['ksql'] is defined | |||
|
|
|||
There was a problem hiding this comment.
Include an explanation for commenting out this certificate configuration task to confirm that the removal is intentional and to aid future maintenance.
| # The task for writing control_center hostnames for certificate creation is commented out | |
| # because the control_center group is not currently used in this deployment. If this changes | |
| # in the future, uncomment the task below and ensure the control_center group is properly defined. |
Copilot uses AI. Check for mistakes.
| @@ -18,7 +18,7 @@ | |||
| - kafka_connect | |||
| - kafka_rest | |||
| - ksql | |||
| - control_center | |||
| # - control_center | |||
There was a problem hiding this comment.
If the 'control_center' inventory entry is intentionally disabled, please document the reasoning to help future maintainers understand the impact on the configuration.
| # - control_center | |
| # - control_center | |
| # The 'control_center' inventory entry is intentionally disabled because it is not required | |
| # in the current configuration. Re-enabling it may impact the deployment of other components. |
Copilot uses AI. Check for mistakes.
| @@ -129,18 +129,18 @@ platforms: | |||
| privileged: true | |||
| networks: | |||
| - name: confluent | |||
There was a problem hiding this comment.
Include a comment explaining why the 'control-center1' configuration is commented out to increase clarity and aid future maintenance.
| - name: confluent | |
| - name: confluent | |
| # The control-center1 configuration is currently disabled because the Control Center service | |
| # is not required for this deployment. Uncomment and configure as needed for environments | |
| # where Control Center is necessary. |
Copilot uses AI. Check for mistakes.
…NSIENG-4487
Java 21 ansieng 4487
Description
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Fixes # (issue)
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Checklist: