feat(instance): improve instance resource handling and updates

- Add InvalidInstanceID.NotFound error code for 404 exceptions
- Implement instance state handling and requeue logic
- Add support for modifying instance attributes (security groups, API termination, etc.)
- Add helper functions for instance state checks and field updates
- Update E2E tests to verify instance updates and security group modifications
- Set additional fields like SecurityGroupIDs and Monitoring in instance spec
- Improve instance syncing and status conditions

This commit enhances the EC2 instance resource controller by adding better error handling,
state management, and support for modifying various instance attributes. It also includes
improvements to E2E tests and resource syncing.

Author: michaelhtm

apis/v1alpha1/ack-generate-metadata.yaml

@@ -1,13 +1,13 @@
 ack_generate_info:
-  build_date: "2025-09-19T17:24:00Z"
-  build_hash: 6b4211163dcc34776b01da9a18217bac0f4103fd
-  go_version: go1.24.6
-  version: v0.52.0
+  build_date: "2025-09-24T22:10:16Z"
+  build_hash: 5bf1e456e1dfc638d47ab492376335f528c0f455
+  go_version: go1.24.5
+  version: v0.52.0-1-g5bf1e45
 api_directory_checksum: b32f97274be98ca3f4cf5cbf559258210c872946
 api_version: v1alpha1
 aws_sdk_go_version: v1.32.6
 generator_config_info:
-  file_checksum: 381d3f31a88cd00e07717b8957ffb5141218130a
+  file_checksum: c4d680200af90f87dd7578bb694e23381253a180
   original_file_name: generator.yaml
 last_modification:
   reason: API generation

apis/v1alpha1/generator.yaml

@@ -364,6 +364,10 @@ resources:
     update_operation:
       custom_method_name: customUpdateDHCPOptions
   Instance:
+    exceptions:
+      errors:
+        404:
+          code: InvalidInstanceID.NotFound
     fields:
       HibernationOptions:
         late_initialize: {}

generator.yaml

@@ -364,6 +364,10 @@ resources:
     update_operation:
       custom_method_name: customUpdateDHCPOptions
   Instance:
+    exceptions:
+      errors:
+        404:
+          code: InvalidInstanceID.NotFound
     fields:
       HibernationOptions:
         late_initialize: {}

pkg/resource/instance/hooks.go

@@ -16,15 +16,28 @@ package instance
 import (
 	"context"
 	"errors"
+	"fmt"
+	"time"
 
 	ackcompare "github.com/aws-controllers-k8s/runtime/pkg/compare"
+	ackrequeue "github.com/aws-controllers-k8s/runtime/pkg/requeue"
 	ackrtlog "github.com/aws-controllers-k8s/runtime/pkg/runtime/log"
+	"github.com/aws/aws-sdk-go-v2/aws"
 	svcsdk "github.com/aws/aws-sdk-go-v2/service/ec2"
 	svcsdktypes "github.com/aws/aws-sdk-go-v2/service/ec2/types"
 
+	"github.com/aws-controllers-k8s/ec2-controller/apis/v1alpha1"
 	"github.com/aws-controllers-k8s/ec2-controller/pkg/tags"
 )
 
+const (
+	stateRunning    = "running"
+	stateStopped    = "stopped"
+	stateTerminated = "terminated"
+
+	requeueUntilReadyDuration = 10 * time.Second
+)
+
 // addInstanceIDsToTerminateRequest populates the list of InstanceIDs
 // in the TerminateInstances request with the resource's InstanceID
 // Return error to indicate to callers that the resource is not yet created.
@@ -53,19 +66,115 @@ func (rm *resourceManager) customUpdateInstance(
 	// an error, then the update was successful and desired.Spec
 	// (now updated.Spec) reflects the latest resource state.
 	updated = rm.concreteResource(desired.DeepCopy())
+	updated.SetStatus(latest)
 
 	if delta.DifferentAt("Spec.Tags") {
 		if err := tags.Sync(
 			ctx, rm.sdkapi, rm.metrics, *latest.ko.Status.InstanceID,
 			desired.ko.Spec.Tags, latest.ko.Spec.Tags,
 		); err != nil {
-			return nil, err
+			return updated, err
+		}
+	}
+
+	if !delta.DifferentExcept("Spec.Tags") {
+		return updated, nil
+	}
+
+	if !isRunning(updated.ko) {
+		return updated, ackrequeue.NeededAfter(
+			fmt.Errorf("requeuing until state is %s or %s", stateRunning, stateStopped),
+			requeueUntilReadyDuration,
+		)
+	}
+
+	input := &svcsdk.ModifyInstanceAttributeInput{
+		InstanceId: latest.ko.Status.InstanceID,
+	}
+	if delta.DifferentAt("Spec.DisableAPITermination") {
+		input.Attribute = "disableApiTermination"
+		input.DisableApiTermination = &svcsdktypes.AttributeBooleanValue{Value: desired.ko.Spec.DisableAPITermination}
+	} else if delta.DifferentAt("Spec.InstanceType") {
+		input.Attribute = "instanceType"
+		input.InstanceType = &svcsdktypes.AttributeValue{Value: desired.ko.Spec.InstanceType}
+	} else if delta.DifferentAt("Spec.KernelID") {
+		input.Attribute = "kernel"
+		input.Kernel = &svcsdktypes.AttributeValue{Value: desired.ko.Spec.KernelID}
+	} else if delta.DifferentAt("Spec.RAMDiskID") {
+		input.Attribute = "ramdisk"
+		input.Ramdisk = &svcsdktypes.AttributeValue{Value: desired.ko.Spec.RAMDiskID}
+	} else if delta.DifferentAt("Spec.InstanceInitiatedShutdownBehavior") {
+		input.Attribute = "instanceInitiatedShutdownBehavior"
+		input.InstanceInitiatedShutdownBehavior = &svcsdktypes.AttributeValue{Value: desired.ko.Spec.InstanceInitiatedShutdownBehavior}
+	} else if delta.DifferentAt("Spec.UserData") {
+		input.Attribute = "userData"
+		input.UserData = &svcsdktypes.BlobAttributeValue{Value: []byte(stringOrEmpty(desired.ko.Spec.UserData))}
+	} else if delta.DifferentAt("Spec.EBSOptimized") {
+		input.Attribute = "ebsOptimized"
+		input.EbsOptimized = &svcsdktypes.AttributeBooleanValue{Value: desired.ko.Spec.EBSOptimized}
+	} else if delta.DifferentAt("Spec.DisableAPIStop") {
+		input.Attribute = "disableApiStop"
+		input.DisableApiStop = &svcsdktypes.AttributeBooleanValue{Value: desired.ko.Spec.DisableAPIStop}
+	} else if delta.DifferentAt("Spec.SecurityGroupIDs") {
+		input.Attribute = "groupSet"
+		input.Groups = aws.ToStringSlice(desired.ko.Spec.SecurityGroupIDs)
+	}
+
+	if input.Attribute != "" {
+		_, err := rm.sdkapi.ModifyInstanceAttribute(ctx, input)
+		rm.metrics.RecordAPICall("UPDATE", "ModifyInstanceAttribute", err)
+		if err != nil {
+			return updated, err
 		}
+		return updated, fmt.Errorf("requeuing until all fields are updated")
 	}
 
 	return updated, nil
 }
 
+func isRunning(ko *v1alpha1.Instance) bool {
+	if ko.Status.State == nil || ko.Status.State.Name == nil {
+		return false
+	}
+
+	// NOTE: (michaelhtm) We will count `stopped` as running for now.
+	// TODO: expose annotation to allow users to start/stop instances
+	return *ko.Status.State.Name == stateRunning || *ko.Status.State.Name == stateStopped
+}
+
+func needsRestart(ko *v1alpha1.Instance) bool {
+	if ko.Status.State == nil || ko.Status.State.Name == nil {
+		return false
+	}
+
+	return *ko.Status.State.Name == stateTerminated
+}
+
+func stringOrEmpty(s *string) string {
+	if s == nil {
+		return ""
+	}
+
+	return *s
+}
+
+func setAdditionalFields(instance svcsdktypes.Instance, ko *v1alpha1.Instance) {
+	ko.Spec.SecurityGroupIDs = []*string{}
+	for _, group := range instance.SecurityGroups {
+		ko.Spec.SecurityGroupIDs = append(ko.Spec.SecurityGroupIDs, group.GroupId)
+	}
+
+	if monitoring := instance.Monitoring; monitoring != nil {
+		switch monitoring.State {
+		case svcsdktypes.MonitoringStateDisabled, svcsdktypes.MonitoringStateDisabling:
+			ko.Spec.Monitoring = &v1alpha1.RunInstancesMonitoringEnabled{Enabled: aws.Bool(false)}
+
+		case svcsdktypes.MonitoringStateEnabled, svcsdktypes.MonitoringStatePending:
+			ko.Spec.Monitoring = &v1alpha1.RunInstancesMonitoringEnabled{Enabled: aws.Bool(true)}
+		}
+	}
+}
+
 var computeTagsDelta = tags.ComputeTagsDelta
 
 // updateTagSpecificationsInCreateRequest adds

pkg/resource/instance/sdk.go

@@ -1,3 +1,5 @@
+	setAdditionalFields(resp.Instances[0], ko)
+	
 	toAdd, toDelete := computeTagsDelta(desired.ko.Spec.Tags, ko.Spec.Tags)
 	if len(toAdd) == 0 && len(toDelete) == 0 {
 		// if desired tags and response tags are equal,

templates/hooks/instance/sdk_create_post_set_output.go.tpl

@@ -1,8 +1,16 @@
+	if needsRestart(ko) {
+		return nil, ackerr.NotFound
+	}
+
+	setAdditionalFields(resp.Reservations[0].Instances[0], ko)
+
+	if !isRunning(ko) {
+		ackcondition.SetSynced(&resource{ko}, corev1.ConditionFalse, nil, aws.String("waiting for resource to be running"))
+	}
 
 	toAdd, toDelete := computeTagsDelta(r.ko.Spec.Tags, ko.Spec.Tags)
 	if len(toAdd) == 0 && len(toDelete) == 0 {
 		// if resource's initial tags and response tags are equal,
 		// then assign resource's tags to maintain tag order
 		ko.Spec.Tags = r.ko.Spec.Tags
 	}
-    

templates/hooks/instance/sdk_read_many_post_set_output.go.tpl

@@ -135,7 +135,7 @@ def instance(ec2_client):
 @service_marker
 @pytest.mark.canary
 class TestInstance:
-    def test_create_delete(self, ec2_client, instance):
+    def test_crud(self, ec2_client, instance):
         (ref, cr) = instance
         resource_id = cr["status"]["instanceID"]
 
@@ -156,6 +156,39 @@ def test_create_delete(self, ec2_client, instance):
                     t['Value'] == INSTANCE_TAG_VAL):
                 tag_present = True
         assert tag_present
+        
+        # Check resource synced successfully
+        assert k8s.wait_on_condition(ref, "ACK.ResourceSynced", "True", wait_periods=5)
+
+        # Ensure instance is running
+        cr = k8s.get_resource(ref)
+        assert 'status' in cr
+        assert 'state' in cr['status']
+        assert 'name' in cr['status']['state']
+        assert cr['status']['state']['name'] == 'running'
+        
+        # Update Instance securityGroupID
+        test_vpc = get_bootstrap_resources().SharedTestVPC
+        updates = {
+            "spec": {
+                "securityGroupIDs" [test_vpc.security_group.group_id]
+            }
+        }
+        k8s.patch_custom_resource(ref, updates)
+        time.sleep(MODIFY_WAIT_AFTER_SECONDS)
+
+        # Check resource synced successfully
+        assert k8s.wait_on_condition(ref, "ACK.ResourceSynced", "True", wait_periods=5)
+
+        # Check Instance updated value
+        instance = get_instance(ec2_client, resource_id)
+        assert instance is not None
+        assert 'SecurityGroups' in instance
+        foundSecurityGroup = False
+        for group in instance['SecurityGroups']:
+            if group['GroupId'] == test_vpc.security_group.group_id:
+                foundSecurityGroup = True
+        assert foundSecurityGroup
 
         # Delete k8s resource
         _, deleted = k8s.delete_custom_resource(ref, 2, 5)