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

Add Candlepin pipeline to run puppet-candlepin tests #1712

Merged
merged 1 commit into from
Jan 16, 2024

Conversation

ehelms
Copy link
Member

@ehelms ehelms commented Oct 6, 2023

Starter test pipeline, this will require some changes to puppet-candlepin to allow configuring the repository to test. My goal is to provide this for theforeman/foreman-infra#1937 (comment)

@ehelms
Copy link
Member Author

ehelms commented Oct 6, 2023

@ekohl How would you make https://github.com/theforeman/puppet-candlepin/blob/master/spec/setup_acceptance_node.pp#L20 configurable for running the beaker tests dynamically? I can imagine wanting to set:

  • Candlepin version for a known repository location
  • A different repository URL for testing against different locations (e.g. stage, prod)

@ekohl
Copy link
Member

ekohl commented Oct 7, 2023

I'm (mostly) happy with how puppet-pulpcore does it.

I'm unhappy with the last part: gha-puppet should have a way to do this without having to copy the whole workflow.

My plan to solve that was to first get voxpupuli/puppet_metadata#35 done. That makes it possible to solve this TODO. Then we can consider ways to input those versions to CI.

@ehelms
Copy link
Member Author

ehelms commented Oct 10, 2023

@ekohl Does this look good for a start? theforeman/puppet-candlepin#244

@ehelms ehelms force-pushed the add-candlepin-pipeline branch from bd4db35 to 63e89ea Compare October 17, 2023 00:42
@ehelms ehelms marked this pull request as ready for review October 17, 2023 00:43
@ehelms ehelms force-pushed the add-candlepin-pipeline branch 2 times, most recently from 6747f3a to 002a613 Compare November 2, 2023 23:15
@ehelms ehelms force-pushed the add-candlepin-pipeline branch 2 times, most recently from e098f4e to 32fee10 Compare November 29, 2023 17:55
roles/beaker/tasks/main.yml Outdated Show resolved Hide resolved
@evgeni evgeni force-pushed the add-candlepin-pipeline branch from 32fee10 to 51c235f Compare November 30, 2023 08:24
@evgeni
Copy link
Member

evgeni commented Nov 30, 2023

@ehelms I moved a few things around, to make it even more re-useable, and overall I like it :)

@ehelms ehelms force-pushed the add-candlepin-pipeline branch from 72f1369 to d8ab376 Compare December 1, 2023 19:24
@ehelms ehelms force-pushed the add-candlepin-pipeline branch 2 times, most recently from ef05f8a to f04a440 Compare January 16, 2024 02:50
@ehelms
Copy link
Member Author

ehelms commented Jan 16, 2024

This now relies on theforeman/puppet-candlepin#252 and relies upon two additional variables:

ansible-playbook pipelines/candlepin.yml -e pipeline_os=centos8-stream -e pipeline_version=nightly -e pipeline_beaker_os=centos9 -e pipeline_beaker_os_version=9

@evgeni
Copy link
Member

evgeni commented Jan 16, 2024

Can't we deduct the additional variables from the pipeline_os? We have (today) no good way for passing in additional vars via Jenkins. We could have them in some vars file… but it feels odd given that IMHO all the data should already be present.

beaker_puppet_module: "puppet-candlepin"
beaker_os: "{{ pipeline_beaker_os }}"
beaker_environment:
BEAKER_FACTER_CANDLEPIN_BASEURL: "https://stagingyum.theforeman.org/candlepin/{{ pipeline_version }}/el{{ pipeline_beaker_os_version }}/x86_64"
Copy link
Member

Choose a reason for hiding this comment

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

technically, given this runs on the vagrant box, this could be

Suggested change
BEAKER_FACTER_CANDLEPIN_BASEURL: "https://stagingyum.theforeman.org/candlepin/{{ pipeline_version }}/el{{ pipeline_beaker_os_version }}/x86_64"
BEAKER_FACTER_CANDLEPIN_BASEURL: "https://stagingyum.theforeman.org/candlepin/{{ pipeline_version }}/el{{ ansible_distribution_major_version }}/x86_64"

Copy link
Member Author

Choose a reason for hiding this comment

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

That won't work for testing EL8 and EL9 when the host itself is only EL8. That is why I felt these additional variables had to be introduced.

Copy link
Member

Choose a reason for hiding this comment

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

Agreed. I was working under the assumption we're keeping the current mode where we spin up a VM that matches the target OS.

I think I'm missing some bigger piece how this should be executed by automation.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fair, while not efficient, that is how this workflow works and we could keep that connection and assumption.

@ehelms ehelms force-pushed the add-candlepin-pipeline branch from f04a440 to 08dc077 Compare January 16, 2024 14:12
@ehelms ehelms merged commit b06a0ae into theforeman:master Jan 16, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants