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

(PE-38522) Add install_puppet_agent_pe_promoted_repo_on #277

Merged
merged 1 commit into from
Jun 11, 2024

Conversation

tlehman
Copy link
Contributor

@tlehman tlehman commented Jun 4, 2024

See https://perforce.atlassian.net/browse/PE-38522 for details

These methods were removed from Host with this commit: tlehman/beaker@cdaedad

  • pe_puppet_agent_promoted_package_info_(max|unix|windows)
  • pe_puppet_agent_promoted_package_install_(max|unix|windows)
  • install_puppet_agent_pe_promoted_repo_on

Instead of adding the methods back onto Host (which lives in beaker), I copied these
methods to beaker-pe, and then changed the self reference to an explicit host
reference that is passed in as a parameter.

@tlehman tlehman requested review from a team as code owners June 4, 2024 23:39
lib/beaker-pe/install/pe_utils.rb Outdated Show resolved Hide resolved
lib/beaker-pe/install/pe_utils.rb Outdated Show resolved Hide resolved
@tlehman
Copy link
Contributor Author

tlehman commented Jun 7, 2024

I cleaned up the commit and the whitespace. This passes when I run the beaker tests locally.

Copy link
Contributor

@jpartlow jpartlow left a comment

Choose a reason for hiding this comment

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

The solaris_install_local_package, install_local_package and uncompress_local_package all exist in Beaker::Host modules still. My initial thought is that we should still be using those rather than duplicating the methods here, unless something else is blocking that.

@@ -891,6 +891,245 @@ def config_hosts_for_proxy_access hosts
end
end

# removes the install_puppet_agent_pe_promoted_repo_on method

# Install shared repo of the puppet-agent on the given host(s). Downloaded from
Copy link
Contributor

Choose a reason for hiding this comment

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

This rubydoc block I think got separated from its method.

Copy link
Contributor

@jpartlow jpartlow Jun 10, 2024

Choose a reason for hiding this comment

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

I think you deleted this rubydoc block rather than reattaching it to its method install_puppet_agent_pe_promoted_repo_on, I believe.

@jpartlow
Copy link
Contributor

jpartlow commented Jun 7, 2024

More fundamentally though, if the only place install_puppet_agent_pe_promoted_repo_on() is used is pe_acceptance_tests setup/prep_legacy.rb, and if prep_legacy is no longer needed, then we may not need any of these methods. Ah no, I forgot it is still used by beaker-pe itself here

@tlehman
Copy link
Contributor Author

tlehman commented Jun 7, 2024

The solaris_install_local_package, install_local_package and uncompress_local_package all exist in Beaker::Host modules still. My initial thought is that we should still be using those rather than duplicating the methods here, unless something else is blocking that.

Nothing is blocking that as far as I know. I'll remove the redundancy

@jpartlow
Copy link
Contributor

Noted a few more items above, and then the commit message needs to catch up with the rest of your changes.

@tlehman tlehman force-pushed the PE38522 branch 2 times, most recently from bc4e463 to c1f07fb Compare June 11, 2024 15:57
@jpartlow
Copy link
Contributor

The rubydoc for install_puppet_agent_pe_promoted_repo_on is still missing, but other than that I think it's sorted out.

See https://perforce.atlassian.net/browse/PE-38522 for details

These methods were removed from Host with this commit: tlehman/beaker@cdaedad
 - pe_puppet_agent_promoted_package_info_(mac|unix|windows)
 - pe_puppet_agent_promoted_package_install_(mac|unix)
 - install_puppet_agent_pe_promoted_repo_on

Instead of adding the methods back onto Host (which lives in beaker), I copied these
methods to beaker-pe, and then changed the `self` reference to an explicit `host`
reference that is passed in as a parameter.
Copy link
Contributor

@jpartlow jpartlow left a comment

Choose a reason for hiding this comment

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

👍

@tlehman tlehman merged commit 1db3137 into puppetlabs:main Jun 11, 2024
4 checks passed
@tlehman tlehman deleted the PE38522 branch June 11, 2024 22:28
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.

3 participants