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

fb_network_scripts: start making gated_template tests pass #177

Closed
wants to merge 1 commit into from

Conversation

jaymzh
Copy link
Collaborator

@jaymzh jaymzh commented Sep 7, 2021

They don't pass yet, but this gets them a lot closer. I mostly
made this for @davide125 to take over and finish.

@davide125
Copy link
Member

cc @anitazha who is refactoring this logic for networkd

@anitazha
Copy link
Contributor

Yea I'm working on moving this to fb_helpers so I can reuse it for fb_networkd. Do you mean just the github tests? It does appear to pass if I do use fb_chefspec on the commandline.

@jaymzh
Copy link
Collaborator Author

jaymzh commented Sep 18, 2021

In this case the tests don't pass on GH because - at least in part - they run in a container which doesn't pull in the systemd stuff which then lets all the rest of this crap break.

This was referenced Oct 14, 2021
@facebook-github-bot
Copy link
Contributor

@scottruss has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Copy link
Contributor

@scottruss scottruss left a comment

Choose a reason for hiding this comment

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

Because of other changes that have happened after a rebase this diff only adds the modprobe to the metadata.

@jaymzh
Copy link
Collaborator Author

jaymzh commented Nov 9, 2021

@scottruss still necessary to fix the tests. Are you asking for a rebase, or are you saying you're gonna add that and I should drop this?

@jaymzh jaymzh requested a review from scottruss February 2, 2022 23:49
They don't pass yet, but this gets them a lot closer. I mostly
made this for @davide125 to take over and finish.

Signed-off-by: Phil Dibowitz <phil@ipom.com>
@facebook-github-bot
Copy link
Contributor

@jaymzh has updated the pull request. You must reimport the pull request before landing.

@jaymzh
Copy link
Collaborator Author

jaymzh commented Feb 2, 2022

@scottruss rebased.

@facebook-github-bot
Copy link
Contributor

@davide125 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

# on the execute[load modules] resource in systemd
include_recipe 'fb_systemd'
include_recipe 'fb_modprobe'
include_recipe 'fb_network_scripts'
Copy link
Contributor

Choose a reason for hiding this comment

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

Pulling in a bunch of largely unrelated recipes (and their dependencies) will make this horribly brittle, and make the deps explode for fb_helpers.

Backing up - what's actually breaking here? I can't infer what systemd has to do with this spec. And I'm failing to find the original error which is showing up in GH

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Well the fb_systemd is for:

       expected Chef::Exceptions::UserIDNotFound, got #<NoMethodError: undefined method `fb_systemd_reload' for cookbook: fb_systemd, recipe: reload :Chef::Recipe> with backtrace:

Then, per the comments, once you load fb_systemd, then it want modprobe.

Not sure I remember what network_scripts was for, but my guess is another error popped up after that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Looks like whatever the dep was is gone. will close.

@jaymzh jaymzh closed this Feb 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants