Skip to content

Stabilize Molecule verification and RPM install workflow#712

Open
beri04 wants to merge 15 commits intojenkinsci:masterfrom
beri04:master
Open

Stabilize Molecule verification and RPM install workflow#712
beri04 wants to merge 15 commits intojenkinsci:masterfrom
beri04:master

Conversation

@beri04
Copy link
Contributor

@beri04 beri04 commented Dec 9, 2025

This PR reintroduces the changes from #536 with the corrected Jenkinsfile.

The previous PR was reverted because the Molecule preparation stage was placed
outside of any node block, which breaks the Jenkins pipeline.

Fixes included:

  • The Molecule preparation stage is now correctly inside the podTemplate →
    nodeWithTimeout(POD_LABEL) block.
  • The duplicate outer stage has been removed.
  • All shell steps are now executed inside a proper node/stage context.

This version should run without causing Jenkinsfile validation errors.

@beri04 beri04 requested a review from a team as a code owner December 9, 2025 09:28
@beri04
Copy link
Contributor Author

beri04 commented Dec 9, 2025

@MarkEWaite @lemeurherve All checks of Jenkins has done successfully.
Apologies for delay!

@MarkEWaite
Copy link
Contributor

MarkEWaite commented Dec 9, 2025

@MarkEWaite @lemeurherve All checks of Jenkins has done successfully. Apologies for delay!

The changes in the pull request are not current with the upstream master branch. You need to merge those changes into this pull request, then I'll need to replay the build of the pull request with your modified Jenkinsfile.

If you have a remote named origin that points to your fork and a remote named upstream that points to the jenkinsci repository, then the commands you need to run are:

git fetch --all
git merge upstream/master

This complication is because the pull request was submitted from your master branch instead of being submitted from a new branch based on the master branch. That's one of the reasons that the pull request template has a checkbox to confirm that a new branch is being used for the pull request changes.

@MarkEWaite
Copy link
Contributor

The GitHub UI offered an update button, so I pressed it. I'll reply the job with the new changes to the Jenkinsfile.

@MarkEWaite
Copy link
Contributor

I've labeled it as on-hold because we must not merge it until after the security release is complete tomorrow.

Copy link
Member

@lemeurherve lemeurherve Dec 9, 2025

Choose a reason for hiding this comment

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

Why the new lines in this file?

Is it required for your fix? If not, can you restore the original file please?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for catching that. The changes in molecule.yml were not intentional, they were caused by upstream merge whitespace differences.
I've now restored the file to match upstream exactly.

@MarkEWaite
Copy link
Contributor

Run 4 is the replay on ci.jenkins.io

@MarkEWaite
Copy link
Contributor

Run 6 is the replay with the Jenkinsfile from the pull request after the changes in c168ce0

@MarkEWaite MarkEWaite changed the title Fix Jenkinsfile :- Move Molecule preparation stage inside node block Stabilize Molecule verification and RPM install workflow Dec 14, 2025
@dduportal dduportal self-assigned this Dec 31, 2025
Jenkinsfile Outdated
sh 'make package && python3 -m pytest bin --junitxml target/junit.xml'
sh '''
make package && python3 -m pytest bin --junitxml target/junit.xml
mkdir -p /var/tmp/target/rpm
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why using the /var/tmp directory? It does not look like a standard Linux location.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point — there’s no strong reason to use /var/tmp here, i am changing to jenkins workspace to follow standard ci-conventions and avoid hard-coded filesystem paths.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done @dduportal i changed jenkinsfile

- ansible_distribution_major_version | int >= 42

# Install Temurin 17 JRE for Fedora 42+
- name: Install Temurin 17 JRE for Fedora 42+
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why reverting the JDK major version from 21 (before) to 17?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Jenkins currently supports Java 17 as the required LTS runtime, java 21 is not yet a supported runtime for Jenkins, so installing temurin 21 on fedora 42+ would be inconsistent with jenkins supported java versions.
((Once Jenkins officially supports Java 21, this can be revisited.))

Copy link
Contributor

Choose a reason for hiding this comment

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

The Java support policy for Jenkins shows that Jenkins has supported Java 21 since November 2023. We've just dropped support for Java 17 in Jenkins weekly and will drop support for Java 17 in Jenkins LTS in April 2026.

Copy link
Collaborator

@dduportal dduportal left a comment

Choose a reason for hiding this comment

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

Even when reading #536 body, I'm not sure to understand all the problems tackled by this change.

Could you update the PR body to explain what is / are the issues you are trying to solve please?

Also blocking this PR: could you explain the logic behind (creating/)using the /var/tmp/target directory in the pipeline?

An absolute path does not seem to be a portable way in a CI or dev environment. But I might be missing something in the setup maybe?
I would have expected molecule to use Docker container for testing each test case, so the absolute path would make sense inside these ephemeral containers. But not from the outer host (e.g. the agent). Did I miss something?

@dduportal
Copy link
Collaborator

Hello @beri04 , before continuing spending time on the PR, can you open an issue to describe the problem you would want to solve please?

@beri04
Copy link
Contributor Author

beri04 commented Jan 7, 2026

Thanks for the guidance. I’ve opened an issue to describe the problem and proposed direction:
#732

I’ll wait for feedback there before continuing work on this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants