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

Test add compilation queue and customisations #13230

Merged
merged 2 commits into from
Dec 17, 2024
Merged

Test add compilation queue and customisations #13230

merged 2 commits into from
Dec 17, 2024

Conversation

lbajolet-hashicorp
Copy link
Contributor

These changes have been extracted from the protobuf PR so they can be independently reviewed and merged.

This PR introduces two things:

  1. A way to customise the code of a plugin to compile so we can have a way to change the compilation artifact produced beyond its version. This is required for the protobuf tests, as we wrote tests using both versions of the SDK in order to test the loading logic for Packer core with different versions of the SDK. Since we add protobuf support on the SDK with the related PR, this means we must have a way to compile versions of the tester plugin with either version of the SDK.
  2. Introduce a global compilation queue: in the current state, plugins are compiled concurrently if they're registered simultaneously. This works well when the code is stable, but because of the introduced change before this one, we are now possibly compiling a plugin with changes in its code. While for a single test we are reasonably able to do this without causing conflicts, this becomes more of a problem when we have multiple test suites running at the same time, each compiling their own versions of the plugin. In this case, we ran into non-deterministic failures, which this approach mitigates.

Related to: #13120

@lbajolet-hashicorp
packer_test: add build customisation capabilities
acb5af5 
When building a plugin, we may want some customisation capabilities
beyond changing the version/pre-release/metadata, and instead run
commands or change files on the filesystem.

To do so, we introduce functions under the BuildCustomisation type,
which have two responsabilities: changing the current state of the
plugin's directory, and cleaning up afterwards.
These customisations are passed as parameters to the BuildSimplePlugin
function, and are called one-by-one, deferring their cleanup after the
build process is finished.

A first implementation of such a customisation is added with this
commit, in order to change the version of a module that the plugin
depends on, which we'll use to change the version of the plugin SDK in
order to test how Packer behaves with different versions of the SDK for
a single plugin.
@lbajolet-hashicorp lbajolet-hashicorp requested a review from a team as a code owner December 16, 2024 16:37
mogrogan
mogrogan requested changes Dec 16, 2024
View reviewed changes
packer_test/common/plugin.go Outdated Show resolved Hide resolved
@lbajolet-hashicorp
packer_test: introduce global compilation queue
cad9cc2 
Compiling plugins was originally intended to be an idempotent operation.
This however starts to change as we introduce build customisations,
which have the unfortunate side-effect of changing the state of the
plugin directory, leading to conflicts between concurrent compilation
jobs.

Therefore to mitigate this problem, this commit changes how compilation
jobs are processed, by introducing a global compilation queue, and
processing plugins' compilation one-by-one from this queue.

This however makes such requests asynchronous, so test suites that
require plugins to be compiled will now have to wait on their completion
before they can start their tests.

To this effect, we introduce one more convenience function that
processes those errors, and automatically fails the test should one
compilation job fail for any reason.
@lbajolet-hashicorp lbajolet-hashicorp force-pushed the test_add_compilation_queue_and_customisations branch from 423582d to cad9cc2 Compare December 16, 2024 18:41
mogrogan
mogrogan approved these changes Dec 16, 2024
View reviewed changes
Copy link
Contributor

@mogrogan mogrogan left a comment

Choose a reason for hiding this comment

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

Thank you for addressing my comment 👍

LGTM

@lbajolet-hashicorp lbajolet-hashicorp merged commit 5ff0f14 into main Dec 17, 2024
11 checks passed
@github-actions GitHub Actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 17, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@mogrogan mogrogan mogrogan approved these changes

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@lbajolet-hashicorp @mogrogan