Merge pull request #51 from aws-controllers-k8s/readiness-check

Add readiness check for resources

api/v1alpha1/resource_group.go

@@ -72,6 +72,8 @@ type Resource struct {
 	Name string `json:"name,omitempty"`
 	// +kubebuilder:validation:Required
 	Definition runtime.RawExtension `json:"definition,omitempty"`
+	// +kubebuilder:validation:Optional
+	ReadyOn []string `json:"readyOn,omitempty"`
 }
 
 // ResourceGroupStatus defines the observed state of ResourceGroup

config/crd/bases/x.symphony.k8s.aws_resourcegroups.yaml

@@ -110,6 +110,11 @@ spec:
                       x-kubernetes-preserve-unknown-fields: true
                     name:
                       type: string
+                    readyOn:
+                      items:
+                        type: string
+                        x-kubernetes-preserve-unknown-fields: true
+                      type: array
                   required:
                   - definition
                   - name

examples/ack-controller/ec2-controller/ec2-controller-instance.yaml

@@ -1,5 +1,5 @@
 apiVersion: x.symphony.k8s.aws/v1alpha1
-kind: EC2controller
+kind: EC2Controller
 metadata:
   name: my-symphony-ec2-controller
   namespace: default

examples/ack-controller/eks-controller/eks-controller-instance.yaml

@@ -1,5 +1,5 @@
 apiVersion: x.symphony.k8s.aws/v1alpha1
-kind: EKScontroller
+kind: EKSController
 metadata:
   name: my-symphony-eks-controller
   namespace: default
@@ -12,7 +12,7 @@ spec:
     deployment: {}
     iamRole:
       oidcProvider: oidc.eks.us-west-2.amazonaws.com/id/50B8942190FBD3A2EF2BF6AB7D27B06B
-    iamRolePolicy: {}
+    iamPolicy: {}
     image:
       resources:
         requests: {}

examples/ack-eks-cluster/eks-cluster.yaml

@@ -19,6 +19,8 @@ spec:
   # resources
   resources:
   - name: clusterVPC
+    readyOn:
+    - ${status.state == "available"}
     definition:
       apiVersion: ec2.services.k8s.aws/v1alpha1
       kind: VPC
@@ -56,6 +58,8 @@ spec:
           - destinationCIDRBlock: 0.0.0.0/0
             gatewayID: ${clusterInternetGateway.status.internetGatewayID}
   - name: clusterSubnetA
+    readyOn:
+    - ${status.state == "available"}
     definition:
       apiVersion: ec2.services.k8s.aws/v1alpha1
       kind: Subnet
@@ -169,6 +173,8 @@ spec:
               ]
           }
   - name: cluster
+    readyOn:
+    - ${status.status == "ACTIVE"}
     definition:
       apiVersion: eks.services.k8s.aws/v1alpha1
       kind: Cluster

examples/deployment-service/deployment-service.yaml

@@ -14,13 +14,15 @@ spec:
       availableReplicas: ${deployment.status.availableReplicas}
   resources:
   - name: deployment
+    readyOn:
+      - ${spec.replicas == status.availableReplicas}
     definition:
       apiVersion: apps/v1
       kind: Deployment
       metadata:
         name: ${spec.name}
       spec:
-        replicas: 1
+        replicas: 50
         selector:
           matchLabels:
             app: deployment

internal/celutil/conversions.go

@@ -48,3 +48,7 @@ func ConvertCELtoGo(v ref.Val) (interface{}, error) {
 		return v.Value(), fmt.Errorf("unsupported type: %v", v.Type())
 	}
 }
+
+func IsBoolType(v ref.Val) bool {
+	return v.Type() == types.BoolType
+}

internal/controller/instance/controller_reconcile.go

@@ -213,6 +213,20 @@ func (igr *InstanceGraphReconciler) reconcileResource(ctx context.Context, resou
 	}
 
 	igr.runtime.SetLatestResource(resourceID, observed)
+
+	log.V(1).Info("Checking if resource is Ready", "resource", resourceID)
+	if ready, err := igr.runtime.IsResourceReady(resourceID); err != nil {
+		log.V(1).Info("Resource not ready", "resource", resourceID)
+		resourceState.State = "WaitingForReadiness"
+		resourceState.Err = fmt.Errorf("resource not ready: %w", err)
+		return igr.delayedRequeue(resourceState.Err)
+	} else if !ready {
+		log.V(1).Info("Resource not ready", "resource", resourceID)
+		resourceState.State = "WaitingForReadiness"
+		resourceState.Err = fmt.Errorf("resource not ready")
+		return igr.delayedRequeue(resourceState.Err)
+	}
+
 	return igr.updateResource(ctx, rc, rUnstructured, observed, resourceID, resourceState)
 }
 

internal/resourcegroup/builder.go

@@ -174,17 +174,28 @@ func (b *GraphBuilder) NewResourceGroup(rg *v1alpha1.ResourceGroup) (*ResourceGr
 				})
 			}
 		}
+		// 6. Parse ReadyOn expressions
+		readyOn, err := parser.ParseConditionExpressions(rgResource.ReadyOn)
+		if err != nil {
+			return nil, fmt.Errorf("failed to parse readyOn expressions: %v", err)
+		}
+
+		readyOnExpressions := []ReadyOnExpression{}
+		for _, e := range readyOn {
+			readyOnExpressions = append(readyOnExpressions, ReadyOnExpression{Resolved: false, Expression: e})
+		}
 
 		_, isNamespaced := namespacedResources[gvk]
 
 		resources[rgResource.Name] = &Resource{
-			ID:               rgResource.Name,
-			GroupVersionKind: gvk,
-			Schema:           resourceSchema,
-			EmulatedObject:   emulatedResource,
-			OriginalObject:   unstructuredResource,
-			Variables:        resourceVariables,
-			Namespaced:       isNamespaced,
+			ID:                 rgResource.Name,
+			GroupVersionKind:   gvk,
+			Schema:             resourceSchema,
+			EmulatedObject:     emulatedResource,
+			OriginalObject:     unstructuredResource,
+			Variables:          resourceVariables,
+			ReadyOnExpressions: readyOnExpressions,
+			Namespaced:         isNamespaced,
 		}
 	}
 
@@ -335,6 +346,48 @@ func (b *GraphBuilder) validateResourceCELExpressions(resources map[string]*Reso
 					return fmt.Errorf("failed to dry-run expression %s: %w", expression, err)
 				}
 			}
+			// validate readyOn Expressions for resource
+			// Only accepting expressions accessing the status and spec for now
+			// and need to evaluate to a boolean type
+			//
+			// TODO(michaelhtm) It shares some of the logic with the loop from above..maybe
+			// we can refactor them or put it in one function.
+			// I would also suggest separating the dryRuns of readyOnExpressions
+			// and the resourceExpressions.
+			for _, expression := range resource.ReadyOnExpressions {
+				fieldNames := getResourceTopLevelFieldNames(resource.Schema)
+				fieldEnv, err := celutil.NewEnvironement(celutil.WithResourceNames(fieldNames))
+				if err != nil {
+					return fmt.Errorf("failed to create CEL environment: %w", err)
+				}
+
+				err = b.validateCELExpressionContext(fieldEnv, expression.Expression, fieldNames)
+				if err != nil {
+					return fmt.Errorf("failed to validate expression context: '%s' %w", expression.Expression, err)
+				}
+				// create context
+				// add resource fields to the context
+				context := map[string]*Resource{}
+				for _, n := range fieldNames {
+					context[n] = &Resource{
+						ID:               n,
+						GroupVersionKind: resource.GroupVersionKind,
+						Schema:           resource.Schema,
+						EmulatedObject: &unstructured.Unstructured{
+							Object: resource.EmulatedObject.Object[n].(map[string]interface{}),
+						},
+					}
+				}
+
+				output, err := b.dryRunExpression(fieldEnv, expression.Expression, context)
+
+				if err != nil {
+					return fmt.Errorf("failed to dry-run expression %s: %w", expression.Expression, err)
+				}
+				if !celutil.IsBoolType(output) {
+					return fmt.Errorf("output of readyOn expression %s can only be of type bool", expression.Expression)
+				}
+			}
 		}
 	}
 
@@ -501,7 +554,7 @@ func (b *GraphBuilder) buildInstanceResource(
 	instanceVariables := []*ResourceVariable{}
 	for _, statusVariable := range statusVariables {
 		// These variables needs to be injected into the status field of the instance.
-		path := ".status" + statusVariable.Path
+		path := ">status" + statusVariable.Path
 		statusVariable.Path = path
 		instanceVariables = append(instanceVariables, &ResourceVariable{
 			Kind:     ResourceVariableKindDynamic,

internal/resourcegroup/ready_on_expression.go

@@ -0,0 +1,48 @@
+// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License"). You may
+// not use this file except in compliance with the License. A copy of the
+// License is located at
+//
+//	http://aws.amazon.com/apache2.0/
+//
+// or in the "license" file accompanying this file. This file 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.
+
+package resourcegroup
+
+import (
+	"slices"
+
+	"k8s.io/kube-openapi/pkg/validation/spec"
+)
+
+type ReadyOnExpression struct {
+	Resolved   bool
+	Expression string
+}
+
+// This function goes through the schema of a resource
+// and retrieves the field names fo rthe readOnly feature
+//
+// Currently we support all the top level fields besides
+// apiVersion and kind. If we do see a case where they would
+// be necessary for readyOns, we can include them here.
+func getResourceTopLevelFieldNames(schema *spec.Schema) []string {
+
+	fieldNames := []string{}
+
+	if schema == nil || schema.Properties == nil {
+		return fieldNames
+	}
+	for k, _ := range schema.Properties {
+		if k != "apiVersion" && k != "kind" {
+			fieldNames = append(fieldNames, k)
+		}
+	}
+
+	slices.Sort(fieldNames)
+	return fieldNames
+}

internal/resourcegroup/resource.go

@@ -35,6 +35,8 @@ type Resource struct {
 
 	Variables []*ResourceVariable
 
+	ReadyOnExpressions []ReadyOnExpression
+
 	Dependencies []string
 	Namespaced   bool
 }

internal/resourcegroup/runtime.go

@@ -104,9 +104,64 @@ func (rt *RuntimeResourceGroup) CanResolveResource(resource string) bool {
 			return false
 		}
 	}
+
 	return true
 }
 
+func (rt *RuntimeResourceGroup) IsResourceReady(resourceID string) (bool, error) {
+
+	expressions := rt.ResourceGroup.Resources[resourceID].ReadyOnExpressions
+	resource := rt.ResolvedResources[resourceID]
+	fieldNames := getResourceTopLevelFieldNames(rt.ResourceGroup.Resources[resourceID].Schema)
+	if len(expressions) == 0 {
+		return true, nil
+	}
+
+	// fieldNames := []string{"status", "spec"}
+	env, err := celutil.NewEnvironement(&celutil.EnvironementOptions{
+		ResourceNames: fieldNames,
+	})
+	// we should not expect errors here since we already compiled it
+	// in the dryRun
+	if err != nil {
+		return false, fmt.Errorf("failed creating new Environment: %w", err)
+	}
+	context := map[string]interface{}{}
+	for _, n := range fieldNames {
+		if obj, ok := resource.Object[n]; ok {
+			context[n] = obj.(map[string]interface{})
+		}
+	}
+	for _, e := range expressions {
+		if e.Resolved {
+			continue
+		}
+		ast, issues := env.Compile(e.Expression)
+		if issues != nil && issues.Err() != nil {
+			return false, fmt.Errorf("failed compiling expression %s: %w", e.Expression, err)
+		}
+		program, err := env.Program(ast)
+		if err != nil {
+			return false, fmt.Errorf("failed programming expression %s: %w", e.Expression, err)
+		}
+
+		output, _, err := program.Eval(context)
+		if err != nil {
+			return false, fmt.Errorf("failed evaluating expression %s: %w", e.Expression, err)
+		}
+		out, err := celutil.ConvertCELtoGo(output)
+		if err != nil {
+			return false, fmt.Errorf("failed converting output %v: %w", output, err)
+		}
+		// keep checking for a false
+		if !out.(bool) {
+			return false, nil
+		}
+		e.Resolved = out.(bool)
+	}
+	return true, err
+}
+
 func (rt *RuntimeResourceGroup) SetLatestResource(name string, resource *unstructured.Unstructured) {
 	rt.ResolvedResources[name] = resource
 }

internal/typesystem/parser/conditions.go

@@ -0,0 +1,45 @@
+// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License"). You may
+// not use this file except in compliance with the License. A copy of the
+// License is located at
+//
+//	http://aws.amazon.com/apache2.0/
+//
+// or in the "license" file accompanying this file. This file 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.
+
+package parser
+
+import (
+	"fmt"
+	"strings"
+)
+
+// This function parses resource condition expressions.
+// These expressions need to be standalone expressions
+// so, this function also does some validation.
+// At the end we return the expressions with '${}' removed
+//
+// To be honest I wouldn't necessarily call it parse, since
+// we are mostly just validating, without caring what's in
+// the expression. Maybe we can rename it in the future 🤔
+func ParseConditionExpressions(conditions []string) ([]string, error) {
+
+	expressions := make([]string, 0, len(conditions))
+
+	for _, e := range conditions {
+		ok, err := isOneShotExpression(e)
+		if err != nil {
+			return nil, err
+		}
+		if !ok {
+			return nil, fmt.Errorf("single expression per line allowed")
+		}
+		expressions = append(expressions, strings.Trim(e, "${}"))
+	}
+
+	return expressions, nil
+}