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

ec2: include attributes in instance observation, fix reconciling tags #2027

Merged

Conversation

justinmir
Copy link

Description of your changes

This ensures that when tags change, the EC2 instance reconciles the update. Previously the ec2 instance controller supported updating tags via CreateTags but did not perform reconciliation when tags changed as it was not included in the check for if the EC2 instance was up to date.

However, the way Update is currently implemented had the implicit assumption that the EC2 instance must be stopped for all updates, which is true for all the ModifyInstanceAttribute calls, however, tags can be applied at runtime. This changes the logic of the controller to (1) include the instance attributes in the instance observations, (2) only attempt to modify instance attributes if the observation does not match the spec.

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

This code has been tested on and deployed in our own fork used in production.

Unit tests are updated to include instance attributes.

This ensures that when tags change, the EC2 instance reconciles the
update. Previously the ec2 instance controller supported updating tags
via `CreateTags` but did not perform reconciliation when tags changed as
it was not included in the check for if the EC2 instance was up to date.

However, the way `Update` is currently implemented had the implicit
assumption that the EC2 instance must be stopped for all updates, which
is true for all the ModifyInstanceAttribute calls, however, tags can be
applied at runtime. This changes the logic of the controller to (1)
include the instance attributes in the instance observations, (2) only
attempt to modify instance attributes if the observation does not match
the spec.

Signed-off-by: justin.miron <justin.miron@reddit.com>
@justinmir justinmir marked this pull request as ready for review April 1, 2024 17:35
Copy link
Collaborator

@MisterMX MisterMX left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you very much @justinmir!

@MisterMX MisterMX merged commit 1fd494d into crossplane-contrib:master Apr 8, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants