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

Fixes #35851 - Add parameter tftp system_image_root #792

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

Fixes #35851 - Add parameter tftp system_image_root #792

wants to merge 1 commit into from

Conversation

bastian-src
Copy link
Contributor

@bastian-src bastian-src commented Dec 12, 2022 •
edited
Loading

Refer to Smart Proxy PR for more information.

Add configurable parameter system_image_root.

@bastian-src bastian-src force-pushed the fix_35851_tftp_system_image branch 4 times, most recently from ff472bb to a02f4bb Compare December 12, 2022 13:15
@bastian-src
Copy link
Contributor Author

bastian-src commented Dec 12, 2022 •
edited
Loading

Need some help to resolve build issue

foreman_proxy::plugin::acd
  on debian-11-x86_64
    with default settings
      is expected to contain Class[foreman_proxy::plugin::remote_execution::script] (FAILED - 2)
      is expected to contain Foreman_proxy::Plugin::Module[acd] (FAILED - 1)
      is expected to contain Foreman_proxy::Plugin::Module[ansible] (FAILED - 3)
      is expected to contain Package[python3-ansible-runner] with ensure => "installed" (FAILED - 4)
      acd.yml should contain the correct configuration (FAILED - 2)

@bastian-src
Copy link
Contributor Author

bastian-src commented Dec 12, 2022 •
edited
Loading

@ekohl do you maybe have an idea why these build errors occur?

ekohl
ekohl requested changes Dec 12, 2022
View reviewed changes
Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

It fails because you enforce that the value is NotUndef when tftp is enabled, but you never provide a value for it. So it fails to compile.

I want to get back to your PR series, but I probably won't have time this week and then my first working day is in the new year.

spec/classes/foreman_proxy__spec.rb Outdated Show resolved Hide resolved
spec/classes/foreman_proxy__tftp_spec.rb Outdated Show resolved Hide resolved
manifests/init.pp Outdated Show resolved Hide resolved
spec/classes/foreman_proxy__spec.rb Outdated Show resolved Hide resolved
@bastian-src
Copy link
Contributor Author

Thanks a lot for your review! It helps a lot.

And thanks also for letting me know the time-frame for my other PRs!

@bastian-src bastian-src force-pushed the fix_35851_tftp_system_image branch 2 times, most recently from a3883c8 to 2ca7b4b Compare December 14, 2022 16:17
@bastian-src
Copy link
Contributor Author

Hey @ekohl! Hope u had a good time and nice holidays. Let me know if you get back to this topic, I'll be happy to help getting the Autoinstall automation feature merged 👍

The main PRs, besides this one, are the following:

@ekohl
Copy link
Member

ekohl commented Feb 2, 2023

@bastian-src sorry, right now I'm swamped with preparing for the Belgian conferences and after that with Foreman 3.6 branching. I wouldn't mind if someone else takes care of reviews but I can't promise anything about myself right now.

@bastian-src
Copy link
Contributor Author

@ekohl Thanks for your response! I guess more developers might be busy until then. Shall I just ping you in 3-5 weeks again?

@bastian-src
Copy link
Contributor Author

@ekohl hey! I've rebased the branches mentioned before and tests are green.
The errors here seem unrelated - I guess we can just re-trigger tests.

Do you have some time for the PRs in the next couple of days or shall we re-schedule?

@bastian-src
Copy link
Contributor Author

@ekohl rebased to latest master :)

Hope you find some time soon 🤞

@bastian-src bastian-src mentioned this pull request Jun 20, 2023
Closed
@bastian-src bastian-src force-pushed the fix_35851_tftp_system_image branch 2 times, most recently from 4f510ef to 64074db Compare July 24, 2023 09:36
@bastian-src @ekohl
Fixes #35851 - Add parameter tftp_system_image_root
a07bafb 
Co-authored-by: Ewoud Kohl van Wijngaarden <ewoud@kohlvanwijngaarden.nl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ekohl ekohl ekohl requested changes

Assignees
No one assigned
Labels
Needs re-review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@bastian-src @ekohl @theforeman-bot