Changed the structure of the cron condition for ISM policy CRD

opensearch-project · Jun 7, 2024 · 836eb0d · 836eb0d
1 parent 4b6b204
commit 836eb0d
Show file tree

Hide file tree

Showing 8 changed files with 65 additions and 35 deletions.
diff --git a/charts/opensearch-operator/files/opensearch.opster.io_opensearchismpolicies.yaml b/charts/opensearch-operator/files/opensearch.opster.io_opensearchismpolicies.yaml
@@ -370,16 +370,26 @@ spec:
                                 description: The cron job that triggers the transition
                                   if no other transition happens first.
                                 properties:
-                                  expression:
-                                    description: The cron expression that triggers
-                                      the transition.
-                                    type: string
-                                  timezone:
-                                    description: The timezone that triggers the transition.
-                                    type: string
+                                  cron:
+                                    description: A wrapper for the cron job that triggers
+                                      the transition if no other transition happens
+                                      first. This wrapper is here to adhere to the
+                                      OpenSearch API.
+                                    properties:
+                                      expression:
+                                        description: The cron expression that triggers
+                                          the transition.
+                                        type: string
+                                      timezone:
+                                        description: The timezone that triggers the
+                                          transition.
+                                        type: string
+                                    required:
+                                    - expression
+                                    - timezone
+                                    type: object
                                 required:
-                                - expression
-                                - timezone
+                                - cron
                                 type: object
                               minDocCount:
                                 description: The minimum document count of the index

diff --git a/opensearch-operator/api/v1/opensearchism_types.go b/opensearch-operator/api/v1/opensearchism_types.go
@@ -251,6 +251,11 @@ type Condition struct {
 }
 
 type Cron struct {
+	// A wrapper for the cron job that triggers the transition if no other transition happens first. This wrapper is here to adhere to the OpenSearch API.
+	CronDetails *CronDetails `json:"cron"`
+}
+
+type CronDetails struct {
 	// The cron expression that triggers the transition.
 	Expression string `json:"expression"`
 	// The timezone that triggers the transition.

diff --git a/opensearch-operator/config/crd/bases/opensearch.opster.io_opensearchismpolicies.yaml b/opensearch-operator/config/crd/bases/opensearch.opster.io_opensearchismpolicies.yaml
@@ -370,16 +370,26 @@ spec:
                                 description: The cron job that triggers the transition
                                   if no other transition happens first.
                                 properties:
-                                  expression:
-                                    description: The cron expression that triggers
-                                      the transition.
-                                    type: string
-                                  timezone:
-                                    description: The timezone that triggers the transition.
-                                    type: string
+                                  cron:
+                                    description: A wrapper for the cron job that triggers
+                                      the transition if no other transition happens
+                                      first. This wrapper is here to adhere to the
+                                      OpenSearch API.
+                                    properties:
+                                      expression:
+                                        description: The cron expression that triggers
+                                          the transition.
+                                        type: string
+                                      timezone:
+                                        description: The timezone that triggers the
+                                          transition.
+                                        type: string
+                                    required:
+                                    - expression
+                                    - timezone
+                                    type: object
                                 required:
-                                - expression
-                                - timezone
+                                - cron
                                 type: object
                               minDocCount:
                                 description: The minimum document count of the index

diff --git a/opensearch-operator/opensearch-gateway/requests/IsmPolicy.go b/opensearch-operator/opensearch-gateway/requests/IsmPolicy.go
@@ -218,7 +218,12 @@ type Condition struct {
 	// The minimum size of the total primary shard storage (not counting replicas) required to transition.
 	MinSize *string `json:"min_size,omitempty"`
 }
+
 type Cron struct {
+	CronDetails *CronDetails `json:"cron"`
+}
+
+type CronDetails struct {
 	// The cron expression that triggers the transition.
 	Expression string `json:"expression"`
 	// The timezone that triggers the transition.

diff --git a/opensearch-operator/opensearch-gateway/services/http_request.go b/opensearch-operator/opensearch-gateway/services/http_request.go
@@ -2,11 +2,12 @@ package services
 
 import (
 	"context"
-	"github.com/opensearch-project/opensearch-go"
-	"github.com/opensearch-project/opensearch-go/opensearchapi"
 	"io"
 	"net/http"
 	"strings"
+
+	"github.com/opensearch-project/opensearch-go"
+	"github.com/opensearch-project/opensearch-go/opensearchapi"
 )
 
 // doHTTPGet performs a HTTP GET request

diff --git a/opensearch-operator/opensearch-gateway/services/os_data_service.go b/opensearch-operator/opensearch-gateway/services/os_data_service.go
@@ -4,9 +4,10 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"strings"
+
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
-	"strings"
 
 	"github.com/Opster/opensearch-k8s-operator/opensearch-operator/opensearch-gateway/requests"
 	"github.com/Opster/opensearch-k8s-operator/opensearch-operator/opensearch-gateway/responses"
@@ -96,9 +97,7 @@ func AppendExcludeNodeHost(service *OsClusterClient, nodeNameToExclude string) (
 		valAsString = strings.Join(valArr, ",")
 	}
 	settings := createClusterSettingsResponseWithExcludeName(valAsString)
-	if err == nil {
-		_, err = service.PutClusterSettings(settings)
-	}
+	_, err = service.PutClusterSettings(settings)
 	return err == nil, err
 }
 
@@ -114,9 +113,7 @@ func RemoveExcludeNodeHost(service *OsClusterClient, nodeNameToExclude string) (
 	valAsString := strings.ReplaceAll(val.(string), nodeNameToExclude, "")
 	valAsString = strings.ReplaceAll(valAsString, ",,", ",")
 	settings := createClusterSettingsResponseWithExcludeName(valAsString)
-	if err == nil {
-		_, err = service.PutClusterSettings(settings)
-	}
+	_, err = service.PutClusterSettings(settings)
 	return err == nil, err
 }
 

diff --git a/opensearch-operator/opensearch-gateway/services/os_ism_service.go b/opensearch-operator/opensearch-gateway/services/os_ism_service.go
@@ -5,14 +5,15 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+
 	"github.com/Opster/opensearch-k8s-operator/opensearch-operator/opensearch-gateway/requests"
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/opensearch-project/opensearch-go/opensearchutil"
 	"sigs.k8s.io/controller-runtime/pkg/log"
 )
 
-var ErrNotFound = errors.New("Policy not found")
+var ErrNotFound = errors.New("policy not found")
 
 // ShouldUpdateISMPolicy checks if the passed policy is same as existing or needs update
 func ShouldUpdateISMPolicy(ctx context.Context, newPolicy, existingPolicy requests.Policy) (bool, error) {
@@ -87,7 +88,7 @@ func UpdateISMPolicy(ctx context.Context, service *OsClusterClient, ismpolicy re
 	}
 	defer resp.Body.Close()
 	if resp.IsError() {
-		return fmt.Errorf("Failed to create ism policy: %s", resp.String())
+		return fmt.Errorf("failed to create ism policy: %s", resp.String())
 	}
 	return nil
 }
@@ -100,7 +101,7 @@ func DeleteISMPolicy(ctx context.Context, service *OsClusterClient, policyName s
 	}
 	defer resp.Body.Close()
 	if resp.IsError() {
-		return fmt.Errorf("Failed to delete ism policy: %s", resp.String())
+		return fmt.Errorf("failed to delete ism policy: %s", resp.String())
 	}
 	return nil
 }
diff --git a/opensearch-operator/pkg/reconcilers/ismpolicy.go b/opensearch-operator/pkg/reconcilers/ismpolicy.go
@@ -219,9 +219,6 @@ func (r *IsmPolicyReconciler) Reconcile() (retResult ctrl.Result, retErr error)
 		r.recorder.Event(r.instance, "Normal", opensearchAPIUpdated, "policy created in opensearch")
 		return ctrl.Result{Requeue: true, RequeueAfter: 30 * time.Second}, retErr
 	}
-	if err != nil {
-		return
-	}
 	priterm := existingPolicy.PrimaryTerm
 	seqno := existingPolicy.SequenceNumber
 	// Reset
@@ -505,8 +502,12 @@ func (r *IsmPolicyReconciler) CreateISMPolicyRequest() (*requests.Policy, error)
 					conditions.MinRolloverAge = transition.Conditions.MinRolloverAge
 				}
 				if transition.Conditions.Cron != nil {
-					conditions.Cron.Expression = transition.Conditions.Cron.Expression
-					conditions.Cron.Timezone = transition.Conditions.Cron.Timezone
+					conditions.Cron = &requests.Cron{
+						CronDetails: &requests.CronDetails{
+							Expression: transition.Conditions.Cron.CronDetails.Expression,
+							Timezone:   transition.Conditions.Cron.CronDetails.Timezone,
+						},
+					}
 				}
 				statename := transition.StateName
 				transitions = append(transitions, requests.Transition{Conditions: conditions, StateName: statename})