Skip to content

Commit

Permalink
Add design decisions
Browse files Browse the repository at this point in the history
Signed-off-by: Gavin Didrichsen <[email protected]>
  • Loading branch information
gavindidrichsen committed Jun 17, 2024
1 parent 0cb0fe5 commit 139f1f3
Show file tree
Hide file tree
Showing 4 changed files with 152 additions and 0 deletions.
1 change: 1 addition & 0 deletions .adr-dir
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
docs/adr
28 changes: 28 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,15 @@ Find a working example of a new-style providers in the [Palo Alto Firewall modul

## [Find the full Resource API documentation here](https://puppet.com/docs/puppet/latest/custom_resources.html)

## Design Decisions

<!-- adrlog -->

* [ADR-0001](docs/adr/0001-test-the-resource-api-again-jruby-as-well-as-ruby-engines.md) - Test the resource api again jruby as well as ruby engines
* [ADR-0002](docs/adr/0002-split-gemfile-dependencies-by-jruby-and-ruby-engines.md) - Split Gemfile dependencies by jruby and ruby engines

<!-- adrlogstop -->

## Related Documents

* The [Resource API specs](https://github.com/puppetlabs/puppet-specifications/blob/master/language/resource-api/README.md) describes details of all the capabilities of this gem.
Expand All @@ -26,8 +35,27 @@ The `puppet-resource_api` gem is part of Puppet from version 6 onward.


## Contributing

We welcome bug reports and pull requests on the Resource API so that we can continue to improve it to the best of our ability. If you would like to contribute, [have a look at our GitHub](https://github.com/puppetlabs/puppet-resource_api) and the [Resource API Backlog](https://github.com/puppetlabs/puppet-resource_api/projects/1).

If making a change to code warrants more explanation, then consider adding a separate design decision document or `ADR`:

* [adr-tools](https://github.com/npryce/adr-tools)
* [adr-log](https://github.com/adr/adr-log)

Follow these steps to add a new design decision and update the README:

```bash
# create a new design decision, e.g., `adr new "Title of your design decision"`
adr new "Do whiteboard wednesday talks"

# fill in the "Context", "Decision", and "Consequences" of the new doc
vim doc/adr/0002-do-whiteboard-wednesday-talks.md

# update the `README.md` table of contents to include the newest design decision
adr-log -i README.md -d doc
```


## Known Issues

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# 1. Test the resource api again jruby as well as ruby engines

Date: 2024-06-17

## Status

Accepted

## Context

Since the Resource API runs inside the puppetserver, then we must test the `resource_api` against the JRuby versions we ship. These require special dependencies to have everything load properly. For example, rubocop 1.48 supports JRuby 9.3+, which includes coverage for versions we support.

## Decision

Therefore, always include `jruby` test environments matching the version included in our puppetserver releases.

## Consequences

`jruby` variants of the `resource_api` in production will be tested in CI.
104 changes: 104 additions & 0 deletions docs/adr/0002-split-gemfile-dependencies-by-jruby-and-ruby-engines.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
# 2. Split Gemfile dependencies by jruby and ruby engines

Date: 2024-06-17

## Status

Accepted

## Context

* Rubocop recently updated it's gems and broke CI across many of our repositories, including the `resource_api`, with an error like the following:

```bash
➜ puppet-resource_api git:(main) bundle exec rubocop
Error: Property AutoCorrect of cop FactoryBot/CreateList is supposed to be a boolean and contextual is not.
+ set +x
➜ puppet-resource_api git:(main)
```

* Updating the `Gemfile` with new rubocop versions like [here](#gemfile-update) fixed all repositories except for `resource_api`.
* The failures on CI were specifically occurriing for the `jruby` runners and looked something like this:

```ruby
➜ puppet_7_jruby_9.3.7.0 git:(cat_1910_fix_nightlies) ✗ bundle install
Fetching gem metadata from https://rubygems.org/.........
Resolving dependencies.....
Bundler found conflicting requirements for the Ruby version:
In Gemfile:
Ruby

ffi (= 1.15.5) was resolved to 1.15.5, which depends on
Ruby (>= 2.3)

puppet was resolved to 7.31.0, which depends on
Ruby (>= 2.5.0)

rubocop (~> 1.64.1) was resolved to 1.64.1, which depends on
Ruby (>= 2.7.0)
➜ puppet_7_jruby_9.3.7.0 git:(cat_1910_fix_nightlies) ✗
```

Before landing on a solution I tried a few different approaches:

* I adjusted the rubocop versions aiming to get one set that would work for all environments (`jruby 9.3.7.0`, `jruby 9.4.2.0`, `ruby 2.7`, and `ruby 3.2`). However, this never worked for me because when I got group working, like `jruby`, then the other group, i,e. `ruby`, broke.
* Second, I tried pinning the rubocop versions to the last working nightly CI versions, but this also failed.

Finally--and to aid in troubleshooting--I tried separating the Gemfile dependencies for `jruby` vs `ruby`. For ruby I kept the latest rubocop versions, but for jruby I kept the versions that worked on the last successful nightly:

```ruby
if RUBY_ENGINE == 'jruby'
gem 'rubocop', '~> 1.48.1', require: false
gem 'rubocop-rspec', '~> 2.20.0', require: false
gem 'rubocop-performance', '~> 1.17.1', require: false
else
gem 'rubocop', '~> 1.64.1', require: false
gem 'rubocop-rspec', '~> 3.0', require: false
gem 'rubocop-performance', '~> 1.16', require: false
end
```

## Decision

Therefore, I decided to use the `if RUBY_ENGINE == 'jruby'` solution to get our CI working again.

## Consequences

The main advantage of this approach is that all the ruby engine environments are now working. As the `jruby` environments are included in our puppet enterprise products it is essential that these continue passing.

One disadvantage is that the `jruby` rubocop is pinned to old versions of rubocop. In other words, as the `ruby` engine versions of rubocop increase then further breakages may occur in the `jruby` environments. For example, I observed that `NewCops` added to the `.rubocop_todo.yml` by my ruby setup, broke the jruby one. The following error occurred on `jruby` setups until I corrected these errors and removed them from the `.rubocop_todo.yml`:

```ruby
➜ puppet_7_jruby_9.3.7.0 git:(cat_1910_fix_nightlies) ✗ be rubocop
+ bundle exec rubocop
Error: unrecognized cop or department RSpec/MetadataStyle found in .rubocop_todo.yml
Did you mean `RSpec/DuplicatedMetadata`?
unrecognized cop or department RSpec/ReceiveMessages found in .rubocop_todo.yml
Did you mean `RSpec/ReceiveNever`?
unrecognized cop or department RSpec/RedundantPredicateMatcher found in .rubocop_todo.yml
Did you mean `RSpec/DuplicatedMetadata`?
unrecognized cop or department Style/RedundantRegexpArgument found in .rubocop_todo.yml
Did you mean `Style/RedundantRegexpEscape`, `Style/RedundantArgument`, `Style/RedundantSelfAssignment`?
unrecognized cop or department Style/SuperArguments found in .rubocop_todo.yml
Did you mean `Style/PerlBackrefs`?
+ set +x
➜ puppet_7_jruby_9.3.7.0 git:(cat_1910_fix_nightlies) ✗
```

## Apendix

### Gemfile update

From this

```ruby
gem 'rubocop', '~> 1.48.1', require: false
gem 'rubocop-rspec', '~> 2.19', require: false
```

to this

```ruby
gem 'rubocop', '~> 1.64.1', require: false
gem 'rubocop-rspec', '~> 3.0', require: false
```

0 comments on commit 139f1f3

Please sign in to comment.