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.

Sign up for GitHub

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

Jump to bottom

Convert Cookbook to Resource-Based #350

Open
wants to merge 102 commits into
base: main
Choose a base branch
from
Open

Convert Cookbook to Resource-Based #350

wants to merge 102 commits into from
+746 −827

Conversation

ArtofBugs
Copy link

Description

Removes recipes and attributes; adds custom resources and helpers. Drops direct support for installation from community repos and from source. See UPGRADING.md for more details on changes.

Issues Resolved

#252

Check List

  • A summary of changes made is included in the CHANGELOG under ## Unreleased
  • New functionality includes testing.
  • New functionality has been documented in the README if applicable.

@ArtofBugs ArtofBugs requested a review from a team as a code owner October 2, 2023 17:57
@ArtofBugs ArtofBugs mentioned this pull request Oct 3, 2023
Closed
6 tasks
@ramereth ramereth marked this pull request as draft October 3, 2023 16:56
@ramereth ramereth added the Release: Major Release to Chef Supermarket as a major change when merged label Oct 3, 2023
@ramereth ramereth linked an issue Oct 3, 2023 that may be closed by this pull request
Custom Resource Rewrite #252
Open
6 tasks
@ArtofBugs ArtofBugs force-pushed the ArtofBugs/php-resources branch 2 times, most recently from c454e6f to fe2f66b Compare October 3, 2023 17:25
damacus
damacus reviewed Oct 3, 2023
View reviewed changes
libraries/helpers.rb Outdated Show resolved Hide resolved
damacus
damacus reviewed Oct 3, 2023
View reviewed changes
libraries/helpers.rb Outdated Show resolved Hide resolved
damacus
damacus reviewed Oct 3, 2023
View reviewed changes
test/cookbooks/test/metadata.rb
@@ -7,3 +7,6 @@

depends 'php'
depends 'apt'
depends 'ondrej_ppa_ubuntu'
depends 'yum-epel'
depends 'yum-remi-chef', '>= 5.0.1'
Copy link
Member

Choose a reason for hiding this comment

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

What's the thinking behind moving these down into the test cookbook?

The original thinking is that if they're needed for it to install, then they're cookbook deps, not test deps.

Copy link
Contributor

Choose a reason for hiding this comment

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

@damacus I'm working with @ArtofBugs on this. The thinking is that the cookbook follow similar principles we have with other cookbooks:

  1. Focus on making it work with the stock OS repos
  2. Remove management of third-party from the cookbook, but make sure the new resource are flexible enough to deal with those third-party packages (thus why we moved them here).

Basically, the third-party repos are causing a lot of issues and really outside the scope of this cookbook.

Copy link
Author

Choose a reason for hiding this comment

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

My thinking was that since these dependencies are only used by the community test recipe (the php_install resource itself doesn't specify where the packages are being installed from), they should be in the test metadata and not the general cookbook metadata. Would it make more sense to move them back?

Copy link
Author

@ArtofBugs ArtofBugs Oct 3, 2023 •
edited

Choose a reason for hiding this comment

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

Oops, sorry for the redundant comment... my GitHub seems to be out of sync a lot today...

Copy link
Member

Choose a reason for hiding this comment

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

Yes, please. Our general thinking is that the cookbook should work off the shelf.

Copy link
Member

Choose a reason for hiding this comment

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

@ramereth if that's our new direction then I'm happy to move that way 👍🏼

Best document it over on sous-chefs.org or something similar. That way we can move a lot of our deps to the test cookbook all over the place.

Copy link
Contributor

Choose a reason for hiding this comment

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

@damacus is the documentation on that site pulled in from the cookbooks automatically?

Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure, I can check and setup automation for it.
mid you haven’t heard back by Monday from me ping me in slack

damacus
damacus reviewed Oct 3, 2023
View reviewed changes
Copy link
Member

@damacus damacus left a comment

Choose a reason for hiding this comment

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

👏🏼 👏🏼 👏🏼 Nice

@damacus
Copy link
Member

damacus commented Nov 8, 2023

@ArtofBugs and @awhittle2 how're you both getting on with this, do you need a hand at all?

@ramereth
Copy link
Contributor

ramereth commented Nov 8, 2023

@damacus @ArtofBugs is working on this for me (they're an OSL student) and is still working out the issues while also refactoring our wrapper cookbook with this (which is uncovering other issues sometimes).

@ArtofBugs ArtofBugs force-pushed the ArtofBugs/php-resources branch 3 times, most recently from 600a651 to abc9e0a Compare April 2, 2024 23:28
@ArtofBugs ArtofBugs marked this pull request as ready for review May 21, 2024 19:34
ArtofBugs and others added 26 commits May 22, 2024 16:35
@ArtofBugs
Update documentation
054457c 
Signed-off-by: ArtofBugs <oweng@osuosl.org>
@ArtofBugs
Disable md-link check on README temporarily; rename master branch to …
6dcea55 
…main branch in all links

Signed-off-by: ArtofBugs <oweng@osuosl.org>
@ArtofBugs
Remove all source install
077bb51 
Signed-off-by: ArtofBugs <oweng@osuosl.org>
@ArtofBugs
Pin Amazon package versions
3d63c64 
Signed-off-by: ArtofBugs <oweng@osuosl.org>
@ArtofBugs
Remove duplicated comment
ba8bcc3 
Signed-off-by: ArtofBugs <oweng@osuosl.org>
@ArtofBugs
Change root_group to just "root"
f4c0261 
Signed-off-by: ArtofBugs <oweng@osuosl.org>
@ArtofBugs
Remove unnecessary helpers code for ext_dir
2a1685d 
Signed-off-by: ArtofBugs <oweng@osuosl.org>
@ArtofBugs
Change documentation links to be relative
42047f5 
Signed-off-by: ArtofBugs <oweng@osuosl.org>
@ArtofBugs
Remove Dangerfile
cf3348b 
Signed-off-by: ArtofBugs <oweng@osuosl.org>
@ArtofBugs
Use php_version helper instead of case statements in helpers; change …
e9ad693 
…php_version to use major-minor only

Signed-off-by: ArtofBugs <oweng@osuosl.org>
@ArtofBugs
Update spec tests
a8a584d 
Signed-off-by: ArtofBugs <oweng@osuosl.org>
@ArtofBugs
Pin amazon php version to 8.2
c0b89ee 
Signed-off-by: ArtofBugs <oweng@osuosl.org>
Signed-off-by: ArtofBugs <74070945+ArtofBugs@users.noreply.github.com>
@ArtofBugs
Include helpers in recipes and resources by default
f7c91a3 
Signed-off-by: ArtofBugs <74070945+ArtofBugs@users.noreply.github.com>
@ArtofBugs
Add UPGRADING.md
1350e0f 
Signed-off-by: ArtofBugs <74070945+ArtofBugs@users.noreply.github.com>
@ArtofBugs
Add clarifications
db4283a 
Signed-off-by: ArtofBugs <74070945+ArtofBugs@users.noreply.github.com>
@ArtofBugs
Add attributes conversion table
c529910 
Signed-off-by: ArtofBugs <74070945+ArtofBugs@users.noreply.github.com>
@ArtofBugs
Update CHANGELOG
670db9a 
Signed-off-by: ArtofBugs <74070945+ArtofBugs@users.noreply.github.com>
@ArtofBugs
Remove dead ticket links
946c142 
Signed-off-by: ArtofBugs <74070945+ArtofBugs@users.noreply.github.com>
@ArtofBugs
Remove unnecessary helpers
03a1d41 
Signed-off-by: ArtofBugs <74070945+ArtofBugs@users.noreply.github.com>
@ArtofBugs
Add note about version helper format change to major-minor only
151f42d 
Signed-off-by: ArtofBugs <74070945+ArtofBugs@users.noreply.github.com>
@ArtofBugs
Fix php-xml version from package manager being ahead on ubuntu/debian
3d596af 
Signed-off-by: ArtofBugs <74070945+ArtofBugs@users.noreply.github.com>
@ArtofBugs
Fix extension location not being found and added to ini
40c03be 
Signed-off-by: ArtofBugs <74070945+ArtofBugs@users.noreply.github.com>
@ArtofBugs
Get most recent stable version by querying the PEAR REST API since th…
c354780 
…e search command ignores preferred state

Signed-off-by: ArtofBugs <74070945+ArtofBugs@users.noreply.github.com>
@ArtofBugs
Fix adding channel name to url
4fbc86e 
Signed-off-by: ArtofBugs <74070945+ArtofBugs@users.noreply.github.com>
@ArtofBugs
Fix missing trailing pipe in md table
560b266 
Signed-off-by: ArtofBugs <74070945+ArtofBugs@users.noreply.github.com>
@ArtofBugs
Search for pear preferred state instead of hardcoding stable
4e846b2 
Signed-off-by: ArtofBugs <74070945+ArtofBugs@users.noreply.github.com>
@ArtofBugs
Update to match kitchen.global.yml supported platforms
5a459ac 
Signed-off-by: ArtofBugs <74070945+ArtofBugs@users.noreply.github.com>
@ArtofBugs
Update versions helper
ed30576 
Signed-off-by: ArtofBugs <74070945+ArtofBugs@users.noreply.github.com>
@ArtofBugs
Update Changelog
f008a83 
Signed-off-by: ArtofBugs <74070945+ArtofBugs@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@damacus damacus damacus left review comments

@ramereth ramereth ramereth left review comments

@xorima xorima xorima left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@ramereth ramereth

@ArtofBugs ArtofBugs

Labels
Release: Major Release to Chef Supermarket as a major change when merged
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Custom Resource Rewrite
5 participants
@ArtofBugs @damacus @ramereth @xorima @awhittle2