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

Fail with guidance if peadm::util::retrieve_and_upload is used on PE XL with the PCP transport #317

Merged
merged 3 commits into from
Feb 1, 2023

Conversation

Sharpie
Copy link
Member

@Sharpie Sharpie commented Jan 26, 2023

This changeset updates peadm::util::retrieve_and_upload to fail if the PCP transport is in use. When PCP is in use, file uploads are done via the bolt_shim::upload task, which essentially reads the file into memory, encodes it, and then ships it over Orchestrator as an argument to the bolt_shim::upload task.

This method of file transfer is utterly inadequate for shipping files as large as the PE installer.

Additionally, the fail_on_transport() function is enhanced to accept an optional error message and existing usages of fail_on_transport() are updated with descriptive messages that explain why PCP is unavailable and how to work around it.

Unit tests for fail_on_transport() are also fully implemented.

@Sharpie Sharpie requested a review from a team as a code owner January 26, 2023 18:41
@Sharpie Sharpie force-pushed the fix-upgrades-with-pcp branch from b373f78 to 5fab565 Compare January 26, 2023 18:41
This commit updates the fail_on_transport_spec.rb
file to contain a fully implemented test instead
of an outline stubbed out with `xif`.
This commit updates the `fail_on_transport` function
with an optional `message` parameter that can be
used to supply an error message.

The `peadm::upgrade` and `peadm::subplans::modify_certificate`
plans are also updated to use this parameter to provide
an explanation of why use of the "pcp" transport is resulting
in a failure along with suggestions for how to configure
the plans for success.
The `peadm::util::retrieve_and_upload` plan downloads a
PE installer tarball and then uploads it to a set of
hosts that need it (Primary and Database nodes).

When the PCP transport is in use, this upload is done
via the `bolt_shim::upload` task, which essentially
reads the file into memory, encodes it, and then
ships it over Orchestrator as a task argument.

This method of file transfer is utterly inadequate
for shipping files as large as the PE installer
which is over half of a gigabyte in size. Use
of "bolt_shim::upload" will cause either a
Linux kernel OOM, an Orchestrator OOM, or
slam into the `max-message-size` allowed by
the Orchestrator.

This commit updates `peadm::util::retrieve_and_upload`
to fail with a descriptive error message if any
of the target nodes is using the PCP transport.
@Sharpie Sharpie force-pushed the fix-upgrades-with-pcp branch from 5fab565 to 5f00ad7 Compare January 26, 2023 19:28
@Sharpie Sharpie changed the title Fail with guidance if peadm::upgrade is used on PE XL with the PCP transport Fail with guidance if peadm::util::retrieve_and_upload is used on PE XL with the PCP transport Jan 27, 2023
@timidri timidri self-assigned this Feb 1, 2023
@timidri timidri self-requested a review February 1, 2023 15:46
@timidri timidri added the feature label Feb 1, 2023
Copy link
Contributor

@timidri timidri left a comment

Choose a reason for hiding this comment

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

LGTM! @Sharpie thanks for the addition!

@timidri timidri merged commit 25335c4 into puppetlabs:main Feb 1, 2023
@Sharpie Sharpie deleted the fix-upgrades-with-pcp branch February 1, 2023 23:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants