Adds basic workflow for pdk based modules #15
Conversation
There was a problem hiding this comment.
Do you have an example repository to test this on? I often use https://github.com/voxpupuli/puppet-example (also for modulesyncs) Perhaps it's better to create a puppet-example-pdk for it.
.github/workflows/pdk-basic.yml
Outdated
| validate: | ||
| # The type of runner that the job will run on | ||
| runs-on: ubuntu-latest | ||
| container: puppet/puppet-dev-tools:2022-03-28-92c7176 |
There was a problem hiding this comment.
You can make this an input so it's only defined once and then callers can override it easily.
There was a problem hiding this comment.
Awesome. Only thing I can't do is allow user to pass in matrix. Not sure if people want to support 5,6,7 and also need to control when pdk stops supporting 5.
logicminds
force-pushed
the
pdk
branch
3 times, most recently
from
1e4ab68 to
1254451
Compare
|
Here is an example: https://github.com/nwops/kontrol-repo-yaml/actions Note this is for my control repo but it would be the same for component modules too (except ra10ke jobs). I am thinking this might have to be a separate repo if it should be reusable as an action rather than a workflow. |
.github/workflows/pdk-basic.yml
Outdated
| runs-on: ubuntu-latest | ||
| strategy: | ||
| matrix: | ||
| puppet-version: [5, 6, 7] |
There was a problem hiding this comment.
We have https://github.com/voxpupuli/gha-puppet/blob/v1/.github/workflows/basic.yml#L55-L57 that gets all major puppet versions from the metadata.json and build the matrix dynamically. I'm wondering if this could be ported to here and if it would be worth the effort.
There was a problem hiding this comment.
That is neat tool. However, that won't work at the enterprise level where metadata is unreliable.
There was a problem hiding this comment.
If you're going to specify it somewhere, why not encourage people to do it in metadata.json rather than in workflow.yaml?
There was a problem hiding this comment.
I am going to update the workflow to use the suggested method. My only concern is that metadata doesn't really support future versions of puppet. So if a modules is only compatible with puppet 6,7 how do we test for 8 before the metadata is updated to 8?
I also filed: puppetlabs/puppet-dev-tools#113 to add puppet-metadata
There was a problem hiding this comment.
So if a modules is only compatible with puppet 6,7 how do we test for 8 before the metadata is updated to 8?
It should handle that well since it iterates the major versions range:
https://github.com/voxpupuli/puppet_metadata/blob/bda0b0f62782492f2f8d5d3392e8aa6d02faff72/lib/puppet_metadata/metadata.rb#L138-L151
As you can see there, it doesn't handle the unbounded dependency automatically. >= 7.0 will only be tested on 7 until we change the hardcoded upper limit. I didn't know how to solve that otherwise. In practice I also think it's uncommon.
There was a problem hiding this comment.
Also how do I handle the case when the user does not have metadata or does not have the puppet version metadata. I don't know if this would occur. Is there a default value that can be supplied or friendly error to update the metadata?
logicminds
force-pushed
the
pdk
branch
3 times, most recently
from
c0ce802 to
c1b9306
Compare
There was a problem hiding this comment.
In the README we document every workflow (under https://github.com/voxpupuli/gha-puppet#calling-test-workflows). Please document the PDK example in the same way. That allows (potential) users to discover it's even a feature.
.github/workflows/pdk-basic.yml
Outdated
| - uses: actions/checkout@v2 | ||
| - name: action-pdk-validate-puppet-7 | ||
| run: pdk validate --puppet-version=7 | ||
| - run: gem install puppet_metadata -N |
There was a problem hiding this comment.
In scripts I'd always use the long form
| - run: gem install puppet_metadata -N | |
| - run: gem install puppet_metadata --no-document |
| run: eval `ssh-agent -s` && ssh-add - <<< '${{ secrets.PRIVATE_SSH_KEY }}' | ||
| - name: Puppet | ||
| uses: voxpupuli/gha-puppet/.github/workflows/pdk-basic.yml@master |
There was a problem hiding this comment.
I think you're missing indenting
| run: eval `ssh-agent -s` && ssh-add - <<< '${{ secrets.PRIVATE_SSH_KEY }}' | |
| - name: Puppet | |
| uses: voxpupuli/gha-puppet/.github/workflows/pdk-basic.yml@master | |
| run: eval `ssh-agent -s` && ssh-add - <<< '${{ secrets.PRIVATE_SSH_KEY }}' | |
| - name: Puppet | |
| uses: voxpupuli/gha-puppet/.github/workflows/pdk-basic.yml@master |
| jobs: | ||
| puppet: | ||
| - name: Setup deploy key |
There was a problem hiding this comment.
I wonder if it should be a secret that the workflow accepts. Here is where we have an example for the release workflow:
gha-puppet/.github/workflows/release.yml
Lines 17 to 23 in 238b8d0
(see https://github.com/voxpupuli/gha-puppet#calling-the-release-workflow for how to call it):
Then you can add it as a when: secrets.PRIVATE_SSH_KEY in the workflow and you don't need as much duplication in various workflows.
| - name: action-pdk-validate-puppet-7 | ||
| run: pdk validate --puppet-version=7 |
There was a problem hiding this comment.
I'd make version 7 an input with a default as well. That allows users to trivially override it. Also means switching 7 to 8 is just a one line change instead of two.
| - run: gem install puppet_metadata --no-document | ||
| - name: Setup Test Matrix | ||
| id: get-outputs | ||
| run: metadata2gha --use-fqdn |
There was a problem hiding this comment.
The FQDN part is only relevant for Beaker. Doesn't hurt, but it doesn't add anything either. I wonder if I should push it into beaker-hostgenerator itself, perhaps even make it the default.
4 participants
This provides a basic gha workflow for people using pdk. I don't know if this is the correct spot for this workflow but thought I would add in case others want to reference or use.
I am a bit new to gha so this is missing a matrix and extra inputs for further configuration. I don't know if configuration is necessary except for disabling jobs to save on resources.
The docker image
puppet/puppet-dev-tools:2022-03-28-92c7176is the minimum required to make all this work and I don't suggest that the pdk image be used since it is not maintained.