Skip to content

Commit

Permalink
Fix bugs
Browse files Browse the repository at this point in the history
  • Loading branch information
thegridman committed May 12, 2023
1 parent aa9f38a commit 8f3b544
Show file tree
Hide file tree
Showing 10 changed files with 62 additions and 53 deletions.
7 changes: 3 additions & 4 deletions api/v1/coherence_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ var _ webhook.Defaulter = &Coherence{}

// Default implements webhook.Defaulter so a webhook will be registered for the type
func (in *Coherence) Default() {
spec := in.GetStatefulSetSpec()
spec, _ := in.GetStatefulSetSpec()
// set the default replicas if not present
if spec.Replicas == nil {
spec.SetReplicas(spec.GetReplicas())
Expand All @@ -61,9 +61,8 @@ func SetCommonDefaults(in CoherenceResource) {
if status.Phase == "" {
logger.Info("Setting defaults for new resource")

if in.GetType() == CoherenceTypeStatefulSet {
var s interface{} = *spec
stsSpec := s.(CoherenceStatefulSetResourceSpec)
stsSpec, found := in.GetStatefulSetSpec()
if found {
// ensure the operator finalizer is present
if stsSpec.AllowUnsafeDelete != nil && *stsSpec.AllowUnsafeDelete {
if controllerutil.ContainsFinalizer(in, CoherenceFinalizer) {
Expand Down
2 changes: 1 addition & 1 deletion api/v1/coherence_webhook_job.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ var _ webhook.Defaulter = &CoherenceJob{}

// Default implements webhook.Defaulter so a webhook will be registered for the type
func (in *CoherenceJob) Default() {
spec := in.GetJobResourceSpec()
spec, _ := in.GetJobResourceSpec()
coherenceSpec := spec.Coherence
if spec.Coherence == nil {
coherenceSpec = &CoherenceSpec{}
Expand Down
6 changes: 2 additions & 4 deletions api/v1/coherence_webhook_job_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,7 @@ func TestJobShouldAddFinalizer(t *testing.T) {
c := coh.CoherenceJob{}
c.Default()
finalizers := c.GetFinalizers()
g.Expect(len(finalizers)).To(Equal(1))
g.Expect(finalizers).To(ContainElement(coh.CoherenceFinalizer))
g.Expect(len(finalizers)).To(Equal(0))
}

func TestJobShouldNotAddFinalizerAgainIfPresent(t *testing.T) {
Expand All @@ -73,10 +72,9 @@ func TestJobShouldNotRemoveFinalizersAlreadyPresent(t *testing.T) {
}
c.Default()
finalizers := c.GetFinalizers()
g.Expect(len(finalizers)).To(Equal(3))
g.Expect(len(finalizers)).To(Equal(2))
g.Expect(finalizers).To(ContainElement("foo"))
g.Expect(finalizers).To(ContainElement("bar"))
g.Expect(finalizers).To(ContainElement(coh.CoherenceFinalizer))
}

func TestJobDefaultReplicasIsNotOverriddenWhenAlreadySet(t *testing.T) {
Expand Down
9 changes: 7 additions & 2 deletions api/v1/coherencejobresource_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,13 @@ func (in *CoherenceJob) GetSpec() *CoherenceResourceSpec {
}

// GetJobResourceSpec returns this resource's CoherenceJobResourceSpec
func (in *CoherenceJob) GetJobResourceSpec() *CoherenceJobResourceSpec {
return &in.Spec
func (in *CoherenceJob) GetJobResourceSpec() (*CoherenceJobResourceSpec, bool) {
return &in.Spec, true
}

// GetStatefulSetSpec always returns nil and false
func (in *CoherenceJob) GetStatefulSetSpec() (*CoherenceStatefulSetResourceSpec, bool) {
return nil, false
}

func (in *CoherenceJob) AddAnnotation(key, value string) {
Expand Down
6 changes: 6 additions & 0 deletions api/v1/coherenceresource.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,12 @@ type CoherenceResource interface {
IsBeforeVersion(version string) bool
// GetSpec returns this resource's CoherenceResourceSpec
GetSpec() *CoherenceResourceSpec
// GetJobResourceSpec returns this resource's CoherenceJobResourceSpec.
// If the spec is not a CoherenceJobResourceSpec the bool return value will be false.
GetJobResourceSpec() (*CoherenceJobResourceSpec, bool)
// GetStatefulSetSpec returns this resource's CoherenceStatefulSetResourceSpec
// If the spec is not a CoherenceStatefulSetResourceSpec the bool return value will be false.
GetStatefulSetSpec() (*CoherenceStatefulSetResourceSpec, bool)
// GetStatus returns this resource's CoherenceResourceStatus
GetStatus() *CoherenceResourceStatus
// AddAnnotation adds an annotation to this resource
Expand Down
9 changes: 7 additions & 2 deletions api/v1/coherenceresource_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,13 @@ func (in *Coherence) GetSpec() *CoherenceResourceSpec {
}

// GetStatefulSetSpec returns this resource's CoherenceStatefulSetResourceSpec
func (in *Coherence) GetStatefulSetSpec() *CoherenceStatefulSetResourceSpec {
return &in.Spec
func (in *Coherence) GetStatefulSetSpec() (*CoherenceStatefulSetResourceSpec, bool) {
return &in.Spec, true
}

// GetJobResourceSpec always returns nil and false
func (in *Coherence) GetJobResourceSpec() (*CoherenceJobResourceSpec, bool) {
return nil, false
}

// GetStatus returns this resource's CoherenceResourceSpec
Expand Down
6 changes: 2 additions & 4 deletions api/v1/coherenceresourcespec_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -857,10 +857,8 @@ func (in *CoherenceResourceSpec) CreateDefaultEnv(deployment CoherenceResource)
env = append(env, corev1.EnvVar{Name: EnvVarCohIdentity, Value: deployment.GetName() + "@" + deployment.GetNamespace()})
}

if deployment.GetType() == CoherenceTypeStatefulSet {
var s interface{} = *spec
stsSpec := s.(CoherenceStatefulSetResourceSpec)

stsSpec, found := deployment.GetStatefulSetSpec()
if found {
if stsSpec.ResumeServicesOnStartup != nil {
env = append(env, corev1.EnvVar{Name: EnvVarOperatorAllowResume, Value: BoolPtrToString(stsSpec.ResumeServicesOnStartup)})
}
Expand Down
9 changes: 4 additions & 5 deletions controllers/statefulset/probe.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,9 @@ func (in *CoherenceProbe) IsStatusHA(ctx context.Context, deployment coh.Coheren
log.Info("Checking StatefulSet "+sts.Name+" for StatusHA",
"Namespace", deployment.GetNamespace(), "Name", deployment.GetName())

if deployment.GetType() == coh.CoherenceTypeStatefulSet {
c := deployment.(*coh.Coherence)
s := c.GetStatefulSetSpec()
p := s.GetScalingProbe()
spec, found := deployment.GetStatefulSetSpec()
if found {
p := spec.GetScalingProbe()
return in.ExecuteProbe(ctx, deployment, sts, p)
}
return true
Expand Down Expand Up @@ -110,7 +109,7 @@ func (in *CoherenceProbe) SuspendServices(ctx context.Context, deployment coh.Co
}

c := deployment.(*coh.Coherence)
stsSpec := c.GetStatefulSetSpec()
stsSpec, _ := c.GetStatefulSetSpec()

if viper.GetBool(operator.FlagSkipServiceSuspend) {
log.Info("Skipping suspension of Coherence services in StatefulSet "+sts.Name+
Expand Down
43 changes: 21 additions & 22 deletions controllers/statefulset/statefulset_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,7 @@ func (in *ReconcileStatefulSet) ReconcileAllResourceOfKind(ctx context.Context,
if err != nil {
logger.Info("Error updating deployment status", "error", err.Error())
}
c := deployment.(*coh.Coherence)
stsSpec := c.GetStatefulSetSpec()
stsSpec, _ := deployment.GetStatefulSetSpec()
if stsSpec.IsSuspendServicesOnShutdown() {
// we are scaling down to zero and suspend services flag is true, so suspend services
suspended := in.suspendServices(ctx, deployment, stsCurrent)
Expand Down Expand Up @@ -203,24 +202,26 @@ func (in *ReconcileStatefulSet) ReconcileAllResourceOfKind(ctx context.Context,

// execActions executes actions
func (in *ReconcileStatefulSet) execActions(ctx context.Context, sts *appsv1.StatefulSet, deployment coh.CoherenceResource) {
coherenceProbe := CoherenceProbe{
Client: in.GetClient(),
Config: in.GetManager().GetConfig(),
}
spec, found := deployment.GetStatefulSetSpec()
if found {
coherenceProbe := CoherenceProbe{
Client: in.GetClient(),
Config: in.GetManager().GetConfig(),
}

spec := deployment.(*coh.Coherence).GetStatefulSetSpec()
for _, action := range spec.Actions {
if action.Probe != nil {
if ok := coherenceProbe.ExecuteProbe(ctx, deployment, sts, action.Probe); !ok {
log.Info("Action probe execution failed.", "probe", action.Probe)
for _, action := range spec.Actions {
if action.Probe != nil {
if ok := coherenceProbe.ExecuteProbe(ctx, deployment, sts, action.Probe); !ok {
log.Info("Action probe execution failed.", "probe", action.Probe)
}
}
}
if action.Job != nil {
job := buildActionJob(action.Name, action.Job, deployment)
if err := in.GetClient().Create(ctx, job); err != nil {
log.Info("Action job creation failed", "error", err.Error())
} else {
log.Info(fmt.Sprintf("Created action job '%s'", job.Name))
if action.Job != nil {
job := buildActionJob(action.Name, action.Job, deployment)
if err := in.GetClient().Create(ctx, job); err != nil {
log.Info("Action job creation failed", "error", err.Error())
} else {
log.Info(fmt.Sprintf("Created action job '%s'", job.Name))
}
}
}
}
Expand Down Expand Up @@ -413,8 +414,7 @@ func (in *ReconcileStatefulSet) patchStatefulSet(ctx context.Context, deployment

// ensure the Coherence image is present so that we do not patch on a Coherence resource
// from pre-3.1.x that does not have images set
c := deployment.(*coh.Coherence)
deploymentSpec := c.GetStatefulSetSpec()
deploymentSpec, _ := deployment.GetStatefulSetSpec()
if deploymentSpec.Image == nil {
cohImage := in.GetCoherenceImage(&desiredPodSpec)
in.SetCoherenceImage(&originalPodSpec, cohImage)
Expand Down Expand Up @@ -519,8 +519,7 @@ func (in *ReconcileStatefulSet) scale(ctx context.Context, deployment coh.Cohere
logger := in.GetLog().WithValues("Namespace", deployment.GetNamespace(), "Name", deployment.GetName())
logger.Info("Scaling StatefulSet", "Current", current, "Desired", desired)

c := deployment.(*coh.Coherence)
spec := c.GetStatefulSetSpec()
spec, _ := deployment.GetStatefulSetSpec()
policy := spec.GetEffectiveScalingPolicy()

// ensure that the deployment has a Scaling status
Expand Down
18 changes: 9 additions & 9 deletions pkg/operator/operator_test.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019, 2022 Oracle and/or its affiliates.
* Copyright (c) 2019, 2023, Oracle and/or its affiliates.
* Licensed under the Universal Permissive License v 1.0 as shown at
* http://oss.oracle.com/licenses/upl.
*/
Expand Down Expand Up @@ -36,11 +36,11 @@ func TestShouldCreateV1CRDs(t *testing.T) {
err = mgr.Client.List(ctx, &crdList)
g.Expect(err).NotTo(HaveOccurred())

g.Expect(len(crdList.Items)).To(Equal(1))
g.Expect(len(crdList.Items)).To(Equal(2))

expected := map[string]bool{
"coherence.coherence.oracle.com": false,
}
expected := make(map[string]bool)
expected["coherence.coherence.oracle.com"] = false
expected["coherencejob.coherence.oracle.com"] = false

for _, crd := range crdList.Items {
expected[crd.Name] = true
Expand All @@ -62,9 +62,9 @@ func TestShouldUpdateV1CRDs(t *testing.T) {
err = crdv1.AddToScheme(mgr.Scheme)
g.Expect(err).NotTo(HaveOccurred())

oldCRDs := map[string]*crdv1.CustomResourceDefinition{
"coherence.coherence.oracle.com": nil,
}
oldCRDs := make(map[string]*crdv1.CustomResourceDefinition)
oldCRDs["coherence.coherence.oracle.com"] = nil
oldCRDs["coherencejob.coherence.oracle.com"] = nil

for name := range oldCRDs {
crd := crdv1.CustomResourceDefinition{}
Expand All @@ -84,7 +84,7 @@ func TestShouldUpdateV1CRDs(t *testing.T) {
err = mgr.Client.List(ctx, &crdList)
g.Expect(err).NotTo(HaveOccurred())

g.Expect(len(crdList.Items)).To(Equal(1))
g.Expect(len(crdList.Items)).To(Equal(2))

for _, crd := range crdList.Items {
oldCRD := oldCRDs[crd.Name]
Expand Down

0 comments on commit 8f3b544

Please sign in to comment.