Fix Delete steps

.github/workflows/e2e.yml

@@ -28,19 +28,27 @@ jobs:
     runs-on: pdx01-arc-runners
     if: ${{ github.event.workflow_run.conclusion == 'success' }} && ${{ github.event.workflow_run.event == 'push' }}
     steps:
-    - uses: actions/checkout@v4
-      name: Checkout code
+    - name: Checkout code
+      uses: actions/checkout@v4
+
     - name: Install Go
       uses: actions/setup-go@v5
       with:
         go-version: 'stable'
         check-latest: true
+
     - name: Install dependencies
-      run: sudo apt-get update && sudo apt-get install -y make
+      run: |
+        sudo apt-get update
+        sudo apt-get install -y make
+        make ginkgo
+
     - name: Run e2e-aws tests
-      run: make -f tests/Makefile e2e-aws
+      run: ./hack/e2e_tests.sh aws
+
     - name: Run e2e-vsphere tests
-      run: make -f tests/Makefile e2e-vsphere
+      run: ./hack/e2e_tests.sh vsphere
+
     - name: Archive test logs
       if: ${{ failure() }}
       uses: actions/upload-artifact@v4

Makefile

@@ -17,6 +17,7 @@ GO_SRC := $(shell find . -type f -name '*.go' -not -path "./vendor/*")
 BINARY_NAME ?= holodeck
 
 VERSION := 0.0.1
+GINKGO_VERSION ?= $(shell $(GO_CMD) list -m -f '{{.Version}}' github.com/onsi/ginkgo/v2)
 
 IMAGE_REGISTRY ?= ghcr.io/arangogutierrez
 IMAGE_TAG_NAME ?= $(VERSION)
@@ -67,3 +68,8 @@ controller-gen: ## Download controller-gen locally if necessary.
 .PHONY: manifests
 manifests: controller-gen
 	$(CONTROLLER_GEN) $(CRD_OPTIONS) rbac:roleName=manager-role webhook paths="./..." output:crd:artifacts:config=config/crd/bases
+
+GINKGO = $(PROJECT_DIR)/bin/ginkgo
+.PHONY: ginkgo
+ginkgo: ## Download ginkgo locally if necessary.
+	@GOBIN=$(PROJECT_DIR)/bin GO111MODULE=on $(GO_CMD) install github.com/onsi/ginkgo/v2/ginkgo@$(GINKGO_VERSION)

hack/e2e_tests.sh

@@ -0,0 +1,45 @@
+#!/usr/bin/env bash
+
+# Copyright 2022 The Kubernetes Authors.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+set -o errexit
+set -o nounset
+set -o pipefail
+
+SOURCE_DIR="$(cd "$(dirname -- "${BASH_SOURCE[0]}")" && pwd -P)"
+ROOT_DIR="$SOURCE_DIR/.."
+
+GINKGO="$ROOT_DIR"/bin/ginkgo
+GINKGO_ARGS=${GINKGO_ARGS:-}
+
+CI=${CI:-"true"}
+
+LOG_ARTIFACT_DIR=${LOG_ARTIFACT_DIR:-${ROOT_DIR}/e2e_logs}
+ENV_FILE=${ENV_FILE:-}
+GINKGO_FOCUS=${GINKGO_FOCUS:-}
+
+if [ "$1" == "aws" ]; then
+  ENV_FILE=${ROOT_DIR}/tests/test_aws.yml
+  GINKGO_FOCUS=${GINKGO_FOCUS:-"AWS"}
+elif [ "$1" == "vsphere" ]; then
+  ENV_FILE=${ROOT_DIR}/tests/test_vsphere.yml
+  GINKGO_FOCUS=${GINKGO_FOCUS:-"VSPHERE"}
+fi
+
+# Set all ENV variables for e2e tests
+export LOG_ARTIFACT_DIR ENV_FILE GINKGO_FOCUS CI
+
+# shellcheck disable=SC2086
+$GINKGO $GINKGO_ARGS -v  --focus $GINKGO_FOCUS ./tests/...

pkg/provider/aws/create.go

@@ -30,34 +30,38 @@ import (
 // VPC, Subnet, Internet Gateway, Route Table, Security Group
 func (p *Provider) Create() error {
 	cache := new(AWS)
-	defer p.dumpCache(cache)
 
 	p.updateProgressingCondition(*p.Environment.DeepCopy(), cache, "v1alpha1.Creating", "Creating AWS resources")
 
 	if err := p.createVPC(cache); err != nil {
 		p.updateDegradedCondition(*p.Environment.DeepCopy(), cache, "v1alpha1.Creating", "Error creating VPC")
 		return fmt.Errorf("error creating VPC: %v", err)
 	}
+	p.updateProgressingCondition(*p.Environment.DeepCopy(), cache, "v1alpha1.Creating", "VPC created")
 
 	if err := p.createSubnet(cache); err != nil {
 		p.updateDegradedCondition(*p.Environment.DeepCopy(), cache, "v1alpha1.Creating", "Error creating subnet")
 		return fmt.Errorf("error creating subnet: %v", err)
 	}
+	p.updateProgressingCondition(*p.Environment.DeepCopy(), cache, "v1alpha1.Creating", "Subnet created")
 
 	if err := p.createInternetGateway(cache); err != nil {
 		p.updateDegradedCondition(*p.Environment.DeepCopy(), cache, "v1alpha1.Creating", "Error creating Internet Gateway")
 		return fmt.Errorf("error creating Internet Gateway: %v", err)
 	}
+	p.updateProgressingCondition(*p.Environment.DeepCopy(), cache, "v1alpha1.Creating", "Internet Gateway created")
 
 	if err := p.createRouteTable(cache); err != nil {
 		p.updateDegradedCondition(*p.Environment.DeepCopy(), cache, "v1alpha1.Creating", "Error creating route table")
 		return fmt.Errorf("error creating route table: %v", err)
 	}
+	p.updateProgressingCondition(*p.Environment.DeepCopy(), cache, "v1alpha1.Creating", "Route Table created")
 
 	if err := p.createSecurityGroup(cache); err != nil {
 		p.updateDegradedCondition(*p.Environment.DeepCopy(), cache, "v1alpha1.Creating", "Error creating security group")
 		return fmt.Errorf("error creating security group: %v", err)
 	}
+	p.updateProgressingCondition(*p.Environment.DeepCopy(), cache, "v1alpha1.Creating", "Security Group created")
 
 	if err := p.createEC2Instance(cache); err != nil {
 		p.updateDegradedCondition(*p.Environment.DeepCopy(), cache, "v1alpha1.Creating", "Error creating EC2 instance")

pkg/provider/aws/delete.go

@@ -18,6 +18,7 @@ package aws
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"time"
 
@@ -30,7 +31,6 @@ func (p *Provider) Delete() error {
 	if err != nil {
 		return fmt.Errorf("error retrieving cache: %v", err)
 	}
-	p.updateProgressingCondition(*p.Environment.DeepCopy(), cache, "v1alpha1.Destroying", "Destroying AWS resources")
 
 	if err := p.delete(cache); err != nil {
 		return fmt.Errorf("error destroying AWS resources: %v", err)
@@ -41,99 +41,159 @@ func (p *Provider) Delete() error {
 
 func (p *Provider) delete(cache *AWS) error {
 	var err error
+
 	// Delete the EC2 instance
+	p.updateProgressingCondition(*p.Environment.DeepCopy(), cache, "v1alpha1.Destroying", "Deleting EC2 instance")
 	if cache.Instanceid == "" {
 		p.log.Warning("No instance found to delete")
 	} else {
-		terminateInstancesInput := &ec2.TerminateInstancesInput{
-			InstanceIds: []string{cache.Instanceid},
-		}
-		_, err := p.ec2.TerminateInstances(context.Background(), terminateInstancesInput)
-		if err != nil {
-			return fmt.Errorf("error deleting instance: %v", err)
+		// call deleteEC2 3 times to ensure the instance is deleted or until it returns nil
+		for i := 0; i < 3; i++ {
+			err = p.deleteEC2(cache)
+			if err == nil {
+				break
+			}
+
+			if i == 2 {
+				p.updateDegradedCondition(*p.Environment.DeepCopy(), cache, "v1alpha1.Destroying", "Error deleting EC2 instance")
+				return fmt.Errorf("error deleting EC2 instance: %v", err)
+			}
 		}
+	}
 
-		p.log.Wg.Add(1)
-		go p.log.Loading("Waiting for instance %s to be terminated", cache.Instanceid)
-
-		waiterOptions := []func(*ec2.InstanceTerminatedWaiterOptions){
-			func(o *ec2.InstanceTerminatedWaiterOptions) {
-				o.MaxDelay = 10 * time.Minute
-				o.MinDelay = 5 * time.Second
-			},
-		}
-		wait := ec2.NewInstanceTerminatedWaiter(p.ec2, waiterOptions...)
-		if err := wait.Wait(context.Background(), &ec2.DescribeInstancesInput{
-			InstanceIds: []string{cache.Instanceid},
-		}, 10*time.Minute, waiterOptions...); err != nil {
-			p.fail()
-			return fmt.Errorf("error waiting for instance to be terminated: %v", err)
+	// Delete the VPC
+	p.updateProgressingCondition(*p.Environment.DeepCopy(), cache, "v1alpha1.Destroying", "Deleting VPC resources")
+	for i := 0; i < 3; i++ {
+		err = p.deleteVPC(cache)
+		if err == nil {
+			break
 		}
 
-		// Delete the security group
-		deleteSecurityGroup := &ec2.DeleteSecurityGroupInput{
-			GroupId: &cache.SecurityGroupid,
-		}
-		_, err = p.ec2.DeleteSecurityGroup(context.Background(), deleteSecurityGroup)
-		if err != nil {
-			p.fail()
-			return fmt.Errorf("error deleting security group: %v", err)
+		if i == 2 {
+			p.updateDegradedCondition(*p.Environment.DeepCopy(), cache, "v1alpha1.Destroying", "Error deleting VPC resources")
+			return fmt.Errorf("error deleting VPC resources: %v", err)
 		}
+	}
+
+	return nil
+}
 
-		p.done()
+func (p *Provider) deleteEC2(cache *AWS) error {
+	terminateInstancesInput := &ec2.TerminateInstancesInput{
+		InstanceIds: []string{cache.Instanceid},
+	}
+	_, err := p.ec2.TerminateInstances(context.Background(), terminateInstancesInput)
+	if err != nil {
+		return fmt.Errorf("error deleting instance: %v", err)
 	}
 
 	p.log.Wg.Add(1)
-	go p.log.Loading("Deleting VPC resources")
-	// Delete the subnet
-	deleteSubnet := &ec2.DeleteSubnetInput{
-		SubnetId: &cache.Subnetid,
+	go p.log.Loading("Waiting for instance %s to be terminated", cache.Instanceid)
+
+	waiterOptions := []func(*ec2.InstanceTerminatedWaiterOptions){
+		func(o *ec2.InstanceTerminatedWaiterOptions) {
+			o.MaxDelay = 10 * time.Minute
+			o.MinDelay = 5 * time.Second
+		},
 	}
-	_, err = p.ec2.DeleteSubnet(context.Background(), deleteSubnet)
-	if err != nil {
+	wait := ec2.NewInstanceTerminatedWaiter(p.ec2, waiterOptions...)
+	if err := wait.Wait(context.Background(), &ec2.DescribeInstancesInput{
+		InstanceIds: []string{cache.Instanceid},
+	}, 10*time.Minute, waiterOptions...); err != nil {
 		p.fail()
-		return fmt.Errorf("error deleting subnet: %v", err)
+		return fmt.Errorf("error waiting for instance to be terminated: %v", err)
 	}
 
-	// Delete the route tables
-	deleteRouteTable := &ec2.DeleteRouteTableInput{
-		RouteTableId: &cache.RouteTable,
+	// Delete the security group
+	deleteSecurityGroup := &ec2.DeleteSecurityGroupInput{
+		GroupId: &cache.SecurityGroupid,
 	}
-	_, err = p.ec2.DeleteRouteTable(context.Background(), deleteRouteTable)
+	_, err = p.ec2.DeleteSecurityGroup(context.Background(), deleteSecurityGroup)
 	if err != nil {
 		p.fail()
-		return fmt.Errorf("error deleting route table: %v", err)
+		return fmt.Errorf("error deleting security group: %v", err)
 	}
 
-	// Detach the Internet Gateway
-	detachInternetGateway := &ec2.DetachInternetGatewayInput{
-		InternetGatewayId: &cache.InternetGwid,
-		VpcId:             &cache.Vpcid,
+	p.done()
+	return nil
+}
+
+func (p *Provider) deleteVPC(cache *AWS) error {
+	var err error
+
+	// Delete the VPC
+	p.log.Wg.Add(1)
+	go p.log.Loading("Deleting VPC resources")
+	p.updateProgressingCondition(*p.Environment.DeepCopy(), cache, "v1alpha1.Destroying", "Deleting VPC resources")
+	// Delete the subnet
+	if cache.Subnetid == "" {
+		p.log.Warning("No subnet found to delete")
+	} else {
+		deleteSubnet := &ec2.DeleteSubnetInput{
+			SubnetId: &cache.Subnetid,
+		}
+		_, err = p.ec2.DeleteSubnet(context.Background(), deleteSubnet)
+		if err != nil {
+			err = errors.Join(err, fmt.Errorf("error deleting subnet: %v", err))
+		}
 	}
-	_, err = p.ec2.DetachInternetGateway(context.Background(), detachInternetGateway)
-	if err != nil {
-		p.fail()
-		return fmt.Errorf("error detaching Internet Gateway: %v", err)
+
+	// Delete the route tables
+	if cache.RouteTable == "" {
+		p.log.Warning("No route table found to delete")
+	} else {
+		deleteRouteTable := &ec2.DeleteRouteTableInput{
+			RouteTableId: &cache.RouteTable,
+		}
+		_, err = p.ec2.DeleteRouteTable(context.Background(), deleteRouteTable)
+		if err != nil {
+			err = errors.Join(err, fmt.Errorf("error deleting route table: %v", err))
+		}
 	}
 
-	// Delete the Internet Gateway
-	deleteInternetGatewayInput := &ec2.DeleteInternetGatewayInput{
-		InternetGatewayId: &cache.InternetGwid,
+	// Detach the Internet Gateway
+	if cache.InternetGwid == "" {
+		p.log.Warning("No Internet Gateway found to delete")
+	} else {
+		detachInternetGateway := &ec2.DetachInternetGatewayInput{
+			InternetGatewayId: &cache.InternetGwid,
+			VpcId:             &cache.Vpcid,
+		}
+		_, err = p.ec2.DetachInternetGateway(context.Background(), detachInternetGateway)
+		if err != nil {
+			err = errors.Join(err, fmt.Errorf("error detaching Internet Gateway: %v", err))
+		}
 	}
-	_, err = p.ec2.DeleteInternetGateway(context.Background(), deleteInternetGatewayInput)
-	if err != nil {
-		p.fail()
-		return fmt.Errorf("error deleting Internet Gateway: %v", err)
+
+	// Delete the Internet Gateway
+	if cache.InternetGwid == "" {
+		p.log.Warning("No Internet Gateway found to delete")
+	} else {
+		deleteInternetGatewayInput := &ec2.DeleteInternetGatewayInput{
+			InternetGatewayId: &cache.InternetGwid,
+		}
+		_, err = p.ec2.DeleteInternetGateway(context.Background(), deleteInternetGatewayInput)
+		if err != nil {
+			err = errors.Join(err, fmt.Errorf("error deleting Internet Gateway: %v", err))
+		}
 	}
 
 	// Delete the VPC
-	dVpc := &ec2.DeleteVpcInput{
-		VpcId: &cache.Vpcid,
+	if cache.Vpcid == "" {
+		p.log.Warning("No VPC found to delete")
+	} else {
+		dVpc := &ec2.DeleteVpcInput{
+			VpcId: &cache.Vpcid,
+		}
+		_, err = p.ec2.DeleteVpc(context.Background(), dVpc)
+		if err != nil {
+			err = errors.Join(err, fmt.Errorf("error deleting VPC: %v", err))
+		}
 	}
-	_, err = p.ec2.DeleteVpc(context.Background(), dVpc)
+
 	if err != nil {
 		p.fail()
-		return fmt.Errorf("error deleting VPC: %v", err)
+		return err
 	}
 
 	p.done()

pkg/provider/aws/status.go

@@ -43,6 +43,10 @@ func (p *Provider) Status() (string, error) {
 		return "", err
 	}
 
+	if len(env.Status.Conditions) == 0 {
+		return "", nil
+	}
+
 	return env.Status.Conditions[0].Type, nil
 }