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

Adds a puppetfile workflow that uses ra10ke #16

Open
wants to merge 1 commit into
base: v1
Choose a base branch
from

Conversation

logicminds
Copy link

This is similar to #15 but checks the puppetfile. This is geared towards the control repo. The validate job is awesome but for private repos it won't work until authentication can be added. So not sure how to handle that at the moment.

Looking for feedback and any other jobs that should be added.

@logicminds
Copy link
Author

@logicminds logicminds force-pushed the puppetfile branch 4 times, most recently from e727567 to 689e498 Compare April 20, 2022 23:43
@logicminds
Copy link
Author

inputs:
container_image:
description: Image to use when running tests
default: 'puppet/puppet-dev-tools:2022-03-28-92c7176'
Copy link
Member

Choose a reason for hiding this comment

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

do they offer a latest that that's reliable or could we use the default ubuntu images and install ruby + ra10ke? That would probably reduce the maintenance cost a bit. (maybe wait for feedback from @ekohl here)

@bastelfreak
Copy link
Member

thanks for the PR. I think that's a nice addition to the existing jobs. We already have docs for running the jobs on modules in a control repository. Could you extend that and document the new jobs as well? https://github.com/voxpupuli/gha-puppet#working-with-a-subdirectory

Also we probably should add an action here that validates the syntax of the shared actions :D

@@ -0,0 +1,73 @@
# This is a basic workflow to help you get started with Actions
Copy link
Member

Choose a reason for hiding this comment

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

I think a lot of these comments are redundant and would prefer to drop them.

Copy link
Author

Choose a reason for hiding this comment

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

I will drop them on next push.

# run: mkdir /~/.ssh/ && echo githubPubKey >> /~/.ssh/known_hosts
- name: validate Puppetfile
run: rake -f /Rakefile r10k:validate
r10k_check_dups:
Copy link
Member

Choose a reason for hiding this comment

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

Is there are reason you're not using multiple steps but a new job for every step? This means starting a new container every time + a new checkout, which is slower. You could even run rake -f /Rakefile r10k:validate r10k:duplicates r10k:syntax r10k:deprecations in a single job.

Copy link
Author

Choose a reason for hiding this comment

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

Steps do not run in parallel and some of the rake tasks take some time to complete so it could end up taking longer to combine all of them. Using jobs, runs all the rake tasks in parallel which is faster if the jobs take longer. Additionally, each job can be restarted independently.

Without breaking out the jobs I would expect the overall time to either be the same or longer. Although this really depends on the number of jobs.

It does start a new container and when run on unique runners, pulls the container image for every job (not ideal). Which at least happens in parallel. If a custom runner was supplied this container image would be cached.

Comment on lines 70 to 71
- run: mkdir -p /github/home/.cache
name: "Create cache directory"
Copy link
Member

Choose a reason for hiding this comment

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

That this is needed feels like a bug in the tooling. Also, wouldn't this work?

Suggested change
- run: mkdir -p /github/home/.cache
name: "Create cache directory"
- run: mkdir -p $HOME/.cache
name: "Create cache directory"

Copy link
Author

Choose a reason for hiding this comment

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

Correct, I think this job should be removed until the tooling can be fixed as there were thrown exceptions as well. Although maybe it is just my puppetfile

@logicminds
Copy link
Author

I am still looking for some suggestions on how to solve the authentication problem. If a puppet module points to a private repo it would not be able to check out repo to validate. This is due to authentication and a ssh key would be required. In the past a custom runner with the key on the system was utilized but since these are shared runners I can't really do that.

Is there a way to pull from private repos?

@bastelfreak
Copy link
Member

for private repos there are a few options I think. People could use this shared workflow but add a custom step before that configures an ssh key (at least I think that works?). Or we could have an optional parameter where people can provide a key?

Can you elaborate a bit on the use case? people have a Puppetfile with a private ssh repo for one module? And they want to test this on a public runner on GitHub?

@logicminds
Copy link
Author

Most cases I run into with private repos are either custom repos or mirrored forged modules that protect against the supply chain hack. Since they still exist on github or github enterprise most are private.

It is rarely one module and usually 20-150 modules. I would say 80%+ of my customers are setup this way.

Since github actions are new to enterprises, testing private repos on public runners is convenient ATM. But long term private runners are the way to go. I had the same problems with Gitlab too.

Cloning private repos listed in the Puppetfile presents the same problem the .fixtures.yml file with private repos has. While 80%+ of customers are setup like this, a small number of those are doing unit testing and smaller are using ra10ke.

@ekohl
Copy link
Member

ekohl commented Apr 22, 2022

Don't GitHub actions get an API token you can use?

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.

3 participants