Skip to content

Commit

Permalink
Modify Operator Conditions (#130)
Browse files Browse the repository at this point in the history 
This PR brings in 2 changes:

1. Introduce a new reason for Unknown status:
Currently, we specify BundleDeploymentFailure or InstallationSuccess as reasons
for an unknown status. This means that we are not sure if the install is successfull or
we aren't sure of a failure either. To make it more accurate, we introduce a new reason
"InstallStatusUnknown" to mean that the status cannot be determined at that particular instant.

2. Rename "BundleDeploymentFailed" to "InstallationUnsuccessful"
This prevents leaking of Rukpak API names. BundleDeployment Failed can be one of the reason
for unsuccessful installation.

Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com>
  • Loading branch information
@varshaprasad96
varshaprasad96 committed Feb 28, 2023
1 parent 88392cb commit c96fef5
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 19 deletions.
12 changes: 7 additions & 5 deletions api/v1alpha1/operator_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,11 @@ const (
// TODO(user): add more Types, here and into init()
TypeReady = "Ready"

ReasonInstallationSucceeded = "InstallationSucceeded"
ReasonResolutionFailed = "ResolutionFailed"
ReasonBundleLookupFailed = "BundleLookupFailed"
ReasonBundleDeploymentFailed = "BundleDeploymentFailed"
ReasonInstallationSucceeded = "InstallationSucceeded"
ReasonResolutionFailed = "ResolutionFailed"
ReasonBundleLookupFailed = "BundleLookupFailed"
ReasonInstallationFailed = "InstallationFailed"
ReasonInstallationStatusUnknown = "InstallationStatusUnknown"
)

func init() {
Expand All @@ -49,7 +50,8 @@ func init() {
ReasonInstallationSucceeded,
ReasonResolutionFailed,
ReasonBundleLookupFailed,
ReasonBundleDeploymentFailed,
ReasonInstallationFailed,
ReasonInstallationStatusUnknown,
)
}

Expand Down
15 changes: 9 additions & 6 deletions controllers/operator_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ func (r *OperatorReconciler) reconcile(ctx context.Context, op *operatorsv1alpha
apimeta.SetStatusCondition(&op.Status.Conditions, metav1.Condition{
Type: operatorsv1alpha1.TypeReady,
Status: metav1.ConditionFalse,
Reason: operatorsv1alpha1.ReasonBundleDeploymentFailed,
Reason: operatorsv1alpha1.ReasonInstallationFailed,
Message: err.Error(),
ObservedGeneration: op.GetGeneration(),
})
Expand All @@ -168,7 +168,7 @@ func (r *OperatorReconciler) reconcile(ctx context.Context, op *operatorsv1alpha
apimeta.SetStatusCondition(&op.Status.Conditions, metav1.Condition{
Type: operatorsv1alpha1.TypeReady,
Status: metav1.ConditionUnknown,
Reason: operatorsv1alpha1.ReasonInstallationSucceeded,
Reason: operatorsv1alpha1.ReasonInstallationStatusUnknown,
Message: err.Error(),
ObservedGeneration: op.GetGeneration(),
})
Expand Down Expand Up @@ -320,11 +320,14 @@ func mapBDStatusToReadyCondition(existingBD *rukpakv1alpha1.BundleDeployment, ob
// 3. If the Operator "Ready" status is "False": There is error observed from Rukpak. Update the status accordingly.
status, message := verifyBDStatus(existingBD)
var reason string
// TODO: introduce a new reason for condition Unknown, instead of defaulting it to Installation Succeeded.
if status == metav1.ConditionTrue {

switch status {
case metav1.ConditionTrue:
reason = operatorsv1alpha1.ReasonInstallationSucceeded
} else {
reason = operatorsv1alpha1.ReasonBundleDeploymentFailed
case metav1.ConditionFalse:
reason = operatorsv1alpha1.ReasonInstallationFailed
default:
reason = operatorsv1alpha1.ReasonInstallationStatusUnknown
}

return metav1.Condition{
Expand Down
16 changes: 8 additions & 8 deletions controllers/operator_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ var _ = Describe("Reconcile Test", func() {
cond := apimeta.FindStatusCondition(operator.Status.Conditions, operatorsv1alpha1.TypeReady)
Expect(cond).NotTo(BeNil())
Expect(cond.Status).To(Equal(metav1.ConditionUnknown))
Expect(cond.Reason).To(Equal(operatorsv1alpha1.ReasonBundleDeploymentFailed))
Expect(cond.Reason).To(Equal(operatorsv1alpha1.ReasonInstallationStatusUnknown))
Expect(cond.Message).To(ContainSubstring("waiting for bundleDeployment"))
})
})
Expand Down Expand Up @@ -179,7 +179,7 @@ var _ = Describe("Reconcile Test", func() {
cond := apimeta.FindStatusCondition(operator.Status.Conditions, operatorsv1alpha1.TypeReady)
Expect(cond).NotTo(BeNil())
Expect(cond.Status).To(Equal(metav1.ConditionUnknown))
Expect(cond.Reason).To(Equal(operatorsv1alpha1.ReasonBundleDeploymentFailed))
Expect(cond.Reason).To(Equal(operatorsv1alpha1.ReasonInstallationStatusUnknown))
Expect(cond.Message).To(ContainSubstring("waiting for bundleDeployment"))
})
})
Expand Down Expand Up @@ -227,7 +227,7 @@ var _ = Describe("Reconcile Test", func() {
cond := apimeta.FindStatusCondition(operator.Status.Conditions, operatorsv1alpha1.TypeReady)
Expect(cond).NotTo(BeNil())
Expect(cond.Status).To(Equal(metav1.ConditionUnknown))
Expect(cond.Reason).To(Equal(operatorsv1alpha1.ReasonBundleDeploymentFailed))
Expect(cond.Reason).To(Equal(operatorsv1alpha1.ReasonInstallationStatusUnknown))
Expect(cond.Message).To(ContainSubstring("waiting for bundleDeployment"))
})
})
Expand Down Expand Up @@ -363,7 +363,7 @@ var _ = Describe("Reconcile Test", func() {
cond := apimeta.FindStatusCondition(op.Status.Conditions, operatorsv1alpha1.TypeReady)
Expect(cond).NotTo(BeNil())
Expect(cond.Status).To(Equal(metav1.ConditionUnknown))
Expect(cond.Reason).To(Equal(operatorsv1alpha1.ReasonBundleDeploymentFailed))
Expect(cond.Reason).To(Equal(operatorsv1alpha1.ReasonInstallationStatusUnknown))
Expect(cond.Message).To(ContainSubstring(`waiting for bundleDeployment`))
})

Expand Down Expand Up @@ -393,7 +393,7 @@ var _ = Describe("Reconcile Test", func() {
cond := apimeta.FindStatusCondition(op.Status.Conditions, operatorsv1alpha1.TypeReady)
Expect(cond).NotTo(BeNil())
Expect(cond.Status).To(Equal(metav1.ConditionFalse))
Expect(cond.Reason).To(Equal(operatorsv1alpha1.ReasonBundleDeploymentFailed))
Expect(cond.Reason).To(Equal(operatorsv1alpha1.ReasonInstallationFailed))
Expect(cond.Message).To(ContainSubstring(`failed to unpack`))
})

Expand Down Expand Up @@ -423,7 +423,7 @@ var _ = Describe("Reconcile Test", func() {
cond := apimeta.FindStatusCondition(op.Status.Conditions, operatorsv1alpha1.TypeReady)
Expect(cond).NotTo(BeNil())
Expect(cond.Status).To(Equal(metav1.ConditionFalse))
Expect(cond.Reason).To(Equal(operatorsv1alpha1.ReasonBundleDeploymentFailed))
Expect(cond.Reason).To(Equal(operatorsv1alpha1.ReasonInstallationFailed))
Expect(cond.Message).To(ContainSubstring(`failed to install`))
})

Expand Down Expand Up @@ -490,7 +490,7 @@ var _ = Describe("Reconcile Test", func() {
cond := apimeta.FindStatusCondition(op.Status.Conditions, operatorsv1alpha1.TypeReady)
Expect(cond).NotTo(BeNil())
Expect(cond.Status).To(Equal(metav1.ConditionUnknown))
Expect(cond.Reason).To(Equal(operatorsv1alpha1.ReasonBundleDeploymentFailed))
Expect(cond.Reason).To(Equal(operatorsv1alpha1.ReasonInstallationStatusUnknown))
Expect(cond.Message).To(ContainSubstring(`could not determine the state of bundleDeployment`))
})

Expand Down Expand Up @@ -520,7 +520,7 @@ var _ = Describe("Reconcile Test", func() {
cond := apimeta.FindStatusCondition(op.Status.Conditions, operatorsv1alpha1.TypeReady)
Expect(cond).NotTo(BeNil())
Expect(cond.Status).To(Equal(metav1.ConditionUnknown))
Expect(cond.Reason).To(Equal(operatorsv1alpha1.ReasonBundleDeploymentFailed))
Expect(cond.Reason).To(Equal(operatorsv1alpha1.ReasonInstallationStatusUnknown))
Expect(cond.Message).To(ContainSubstring(`could not determine the state of bundleDeployment`))
})
})
Expand Down

0 comments on commit c96fef5

Please sign in to comment.