Skip to content

Commit

Permalink
Merge pull request #95 from mbaijal/infinite-requeue-2
Browse files Browse the repository at this point in the history
Provide an option to configure a resource to requeue infinitely (Issue #826)
  • Loading branch information
jaypipes authored Jun 24, 2021
2 parents 13160d8 + cb1bbe6 commit 9c35918
Show file tree
Hide file tree
Showing 7 changed files with 157 additions and 5 deletions.
18 changes: 18 additions & 0 deletions pkg/generate/config/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ type ResourceConfig struct {
// very little consistency to the APIs that we can use to instruct the code
// generator :(
UpdateOperation *UpdateOperationConfig `json:"update_operation,omitempty"`
// Reconcile describes options for controlling the reconciliation
// logic for a particular resource.
Reconcile *ReconcileConfig `json:"reconcile,omitempty"`
// UpdateConditionsCustomMethodName provides the name of the custom method on the
// `resourceManager` struct that will set Conditions on a `resource` struct
// depending on the status of the resource.
Expand Down Expand Up @@ -275,6 +278,21 @@ type PrintConfig struct {
OrderBy string `json:"order_by"`
}

// ReconcileConfig describes options for controlling the reconciliation
// logic for a particular resource.
type ReconcileConfig struct {
// RequeueOnSuccessSeconds indicates the number of seconds after which to requeue a
// resource that has been successfully reconciled (i.e. ConditionTypeResourceSynced=true)
// This is useful for resources that are long-lived and may have observable status fields
// change over time that would be useful to refresh those field values for users.
// This field is optional and the default behaviour of the ACK runtime is to not requeue
// resources that have been successfully reconciled. Note that all ACK controllers will
// *flush and resync their watch caches* every 10 hours by default, which will end up
// causing ACK controllers to refresh the status views of all watched resources, but this
// behaviour is expensive and may be turned off in future ACK runtime options.
RequeueOnSuccessSeconds int `json:"requeue_on_success_seconds,omitempty"`
}

// ResourceConfig returns the ResourceConfig for a given named resource
func (c *Config) ResourceConfig(name string) (*ResourceConfig, bool) {
rc, ok := c.Resources[name]
Expand Down
86 changes: 86 additions & 0 deletions pkg/generate/sagemaker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,3 +140,89 @@ func TestSageMaker_Error_Suffix_Message(t *testing.T) {
// Validation Exception has suffix ModelPackageGroup arn:aws:sagemaker:/ does not exist
assert.Equal("&& strings.HasSuffix(awsErr.Message(), \"does not exist.\") ", code.CheckExceptionMessage(crd.Config(), crd, 404))
}

func TestSageMaker_RequeueOnSuccessSeconds(t *testing.T) {
assert := assert.New(t)
require := require.New(t)

g := testutil.NewGeneratorForService(t, "sagemaker")

crds, err := g.GetCRDs()
require.Nil(err)

crd := getCRDByName("Endpoint", crds)
require.NotNil(crd)

// The CreateEndpoint has the following definition:
//
// "CreateEndpoint":{
// "name":"CreateEndpoint",
// "http":{
// "method":"POST",
// "requestUri":"/"
// },
// "input":{"shape":"CreateEndpointInput"},
// "output":{"shape":"CreateEndpointOutput"},
// "errors":[
// {"shape":"ResourceLimitExceeded"}
// ]
// }
//
// Where the CreateEndpointOutput shape looks like this:
//
// "CreateEndpointOutput":{
// "type":"structure",
// "required":["EndpointArn"],
// "members":{
// "EndpointArn":{"shape":"EndpointArn"}
// }
// }
//
// So, we expect that crd.ReconcileRequeuOnSuccessSeconds() returns the requeue
// duration specified in the config file
assert.Equal(10, crd.ReconcileRequeuOnSuccessSeconds())
}

func TestSageMaker_RequeueOnSuccessSeconds_Default(t *testing.T) {
assert := assert.New(t)
require := require.New(t)

g := testutil.NewGeneratorForService(t, "sagemaker")

crds, err := g.GetCRDs()
require.Nil(err)

crd := getCRDByName("DataQualityJobDefinition", crds)
require.NotNil(crd)

// The CreateDataQualityJobDefinition has the following definition:
//
// "CreateDataQualityJobDefinition":{
// "name":"CreateDataQualityJobDefinition",
// "http":{
// "method":"POST",
// "requestUri":"/"
// },
// "input":{"shape":"CreateDataQualityJobDefinitionRequest"},
// "output":{"shape":"CreateDataQualityJobDefinitionResponse"},
// "errors":[
// {"shape":"ResourceLimitExceeded"},
// {"shape":"ResourceInUse"}
// ]
// }
//
// Where the CreateDataQualityJobDefinitionResponse shape looks like this:
//
// "CreateDataQualityJobDefinitionResponse":{
// "type":"structure",
// "required":["JobDefinitionArn"],
// "members":{
// "JobDefinitionArn":{"shape":"MonitoringJobDefinitionArn"}
// }
// }
//
// So, we expect that crd.ReconcileRequeuOnSuccessSeconds() returns the default
// requeue duration of 0 because it is not specified in the config file
assert.Equal(0, crd.ReconcileRequeuOnSuccessSeconds())

}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ resources:
404:
code: ValidationException
message_suffix: does not exist.
Endpoint:
reconcile:
requeue_on_success_seconds: 10
ignore:
resource_names:
- Algorithm
Expand All @@ -35,7 +38,7 @@ ignore:
- Domain
- EdgePackagingJob
- EndpointConfig
- Endpoint
# - Endpoint
- Experiment
- FeatureGroup
- FlowDefinition
Expand Down
18 changes: 18 additions & 0 deletions pkg/model/crd.go
Original file line number Diff line number Diff line change
Expand Up @@ -556,6 +556,24 @@ func (r *CRD) PrintAgeColumn() bool {
return r.cfg.GetResourcePrintAddAgeColumn(r.Names.Camel)
}

// ReconcileRequeuOnSuccessSeconds returns the duration after which to requeue
// the custom resource as int, if specified in generator config.
func (r *CRD) ReconcileRequeuOnSuccessSeconds() int {
if r.cfg == nil {
return 0
}
resGenConfig, found := r.cfg.Resources[r.Names.Original]
if !found {
return 0
}
reconcile := resGenConfig.Reconcile
if reconcile != nil {
return reconcile.RequeueOnSuccessSeconds
}
// handles the default case
return 0
}

// CustomUpdateMethodName returns the name of the custom resourceManager method
// for updating the resource state, if any has been specified in the generator
// config
Expand Down
4 changes: 2 additions & 2 deletions templates/pkg/resource/manager.go.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ func (rm *resourceManager) onError(
r *resource,
err error,
) (acktypes.AWSResource, error) {
r1, updated := rm.updateConditions(r, err)
r1, updated := rm.updateConditions(r, false, err)
if !updated {
return r, err
}
Expand All @@ -204,7 +204,7 @@ func (rm *resourceManager) onError(
func (rm *resourceManager) onSuccess(
r *resource,
) (acktypes.AWSResource, error) {
r1, updated := rm.updateConditions(r, nil)
r1, updated := rm.updateConditions(r, true, nil)
if !updated {
return r, nil
}
Expand Down
10 changes: 10 additions & 0 deletions templates/pkg/resource/manager_factory.go.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,16 @@ func (f *resourceManagerFactory) IsAdoptable() bool {
return {{ .CRD.IsAdoptable }}
}

// RequeueOnSuccessSeconds returns true if the resource should be requeued after specified seconds
// Default is false which means resource will not be requeued after success.
func (f *resourceManagerFactory) RequeueOnSuccessSeconds() int {
{{- if $reconcileRequeuOnSuccessSeconds := .CRD.ReconcileRequeuOnSuccessSeconds }}
return {{ $reconcileRequeuOnSuccessSeconds }}
{{- else }}
return 0
{{- end }}
}

func newResourceManagerFactory() *resourceManagerFactory {
return &resourceManagerFactory{
rmCache: map[string]*resourceManager{},
Expand Down
21 changes: 19 additions & 2 deletions templates/pkg/resource/sdk.go.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ func (rm *resourceManager) setStatusDefaults (
// else it returns nil, false
func (rm *resourceManager) updateConditions (
r *resource,
onSuccess bool,
err error,
) (*resource, bool) {
ko := r.ko.DeepCopy()
Expand All @@ -197,13 +198,17 @@ func (rm *resourceManager) updateConditions (
// Terminal condition
var terminalCondition *ackv1alpha1.Condition = nil
var recoverableCondition *ackv1alpha1.Condition = nil
var syncCondition *ackv1alpha1.Condition = nil
for _, condition := range ko.Status.Conditions {
if condition.Type == ackv1alpha1.ConditionTypeTerminal {
terminalCondition = condition
}
if condition.Type == ackv1alpha1.ConditionTypeRecoverable {
recoverableCondition = condition
}
if condition.Type == ackv1alpha1.ConditionTypeResourceSynced {
syncCondition = condition
}
}

if rm.terminalAWSError(err) {
Expand Down Expand Up @@ -245,15 +250,27 @@ func (rm *resourceManager) updateConditions (
}
}

{{- if $reconcileRequeuOnSuccessSeconds := .CRD.ReconcileRequeuOnSuccessSeconds }}
if syncCondition == nil && onSuccess {
syncCondition = &ackv1alpha1.Condition{
Type: ackv1alpha1.ConditionTypeResourceSynced,
Status: corev1.ConditionTrue,
}
ko.Status.Conditions = append(ko.Status.Conditions, syncCondition)
}
{{- else }}
// Required to avoid the "declared but not used" error in the default case
_ = syncCondition
{{- end }}

{{- if $updateConditionsCustomMethodName := .CRD.UpdateConditionsCustomMethodName }}
// custom update conditions
customUpdate := rm.{{ $updateConditionsCustomMethodName }}(ko, r, err)
if terminalCondition != nil || recoverableCondition != nil || customUpdate {
if terminalCondition != nil || recoverableCondition != nil || syncCondition != nil || customUpdate {
return &resource{ko}, true // updated
}
{{- else }}
if terminalCondition != nil || recoverableCondition != nil {
if terminalCondition != nil || recoverableCondition != nil || syncCondition != nil {
return &resource{ko}, true // updated
}
{{- end }}
Expand Down

0 comments on commit 9c35918

Please sign in to comment.