Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix e2e ms test #375

Closed
wants to merge 2 commits into from
Closed

Fix e2e ms test #375

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9408c5f
feat: set availability to degraded if reconcile fails
sthaha Mar 12, 2024
5a34b04
fix(e2e): ensure model-server is running before checking status
sthaha Mar 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions pkg/api/v1alpha1/kepler_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,10 @@ const (
type ConditionStatus string

const (
ConditionTrue ConditionStatus = "True"
ConditionFalse ConditionStatus = "False"
ConditionUnknown ConditionStatus = "Unknown"
ConditionTrue ConditionStatus = "True"
ConditionFalse ConditionStatus = "False"
ConditionUnknown ConditionStatus = "Unknown"
ConditionDegraded ConditionStatus = "Degraded"
)

type Condition struct {
Expand Down
5 changes: 3 additions & 2 deletions pkg/controllers/kepler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,18 +220,19 @@ func (r KeplerReconciler) setInvalidStatus(ctx context.Context, req ctrl.Request
return nil
}

now := metav1.Now()
invalidKepler.Status.Exporter.Conditions = []v1alpha1.Condition{{
Type: v1alpha1.Reconciled,
Status: v1alpha1.ConditionFalse,
ObservedGeneration: invalidKepler.Generation,
LastTransitionTime: metav1.Now(),
LastTransitionTime: now,
Reason: v1alpha1.InvalidKeplerResource,
Message: "Only a single instance of Kepler named kepler is reconciled",
}, {
Type: v1alpha1.Available,
Status: v1alpha1.ConditionUnknown,
ObservedGeneration: invalidKepler.Generation,
LastTransitionTime: metav1.Now(),
LastTransitionTime: now,
Reason: v1alpha1.InvalidKeplerResource,
Message: "This instance of Kepler is invalid",
}}
Expand Down
50 changes: 29 additions & 21 deletions pkg/controllers/kepler_internal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,31 +189,33 @@ func (r KeplerInternalReconciler) getInternal(ctx context.Context, req ctrl.Requ
}

func (r KeplerInternalReconciler) updateStatus(ctx context.Context, req ctrl.Request, recErr error) error {
logger := r.logger.WithValues("keplerinternal", req.Name, "action", "update-status")
logger.V(3).Info("Start of status update")
defer logger.V(3).Info("End of status update")

return retry.RetryOnConflict(retry.DefaultBackoff, func() error {

logger := r.logger.WithValues("keplerinternal", req.Name)
ki, _ := r.getInternal(ctx, req)
// may be deleted
if ki == nil || !ki.GetDeletionTimestamp().IsZero() {
// retry since some error has occurred
r.logger.V(6).Info("Reconcile has deleted kepler; skipping update")
r.logger.V(6).Info("Reconcile has deleted kepler-internal; skipping status update")
return nil
}

// sanitize the conditions so that all types are present and the order is predictable
ki.Status.Exporter.Conditions = sanitizeConditions(ki.Status.Exporter.Conditions)

updated := r.updateReconciledStatus(ctx, ki, recErr) ||
r.updateAvailableStatus(ctx, ki, recErr)

if !updated {
logger.V(6).Info("no changes to existing status; skipping update")
return nil
}
{
now := metav1.Now()
reconciledChanged := r.updateReconciledStatus(ctx, ki, recErr, now)
availableChanged := r.updateAvailableStatus(ctx, ki, recErr, now)
logger.V(6).Info("conditions updated", "reconciled", reconciledChanged, "available", availableChanged)

now := metav1.Now()
for i := range ki.Status.Exporter.Conditions {
ki.Status.Exporter.Conditions[i].LastTransitionTime = now
if !reconciledChanged && !availableChanged {
logger.V(6).Info("no changes to existing status; skipping update")
return nil
}
}

return r.Client.Status().Update(ctx, ki)
Expand Down Expand Up @@ -256,7 +258,7 @@ func sanitizeConditions(conditions []v1alpha1.Condition) []v1alpha1.Condition {
return conditions
}

func (r KeplerInternalReconciler) updateReconciledStatus(ctx context.Context, ki *v1alpha1.KeplerInternal, recErr error) bool {
func (r KeplerInternalReconciler) updateReconciledStatus(ctx context.Context, ki *v1alpha1.KeplerInternal, recErr error, time metav1.Time) bool {

reconciled := v1alpha1.Condition{
Type: v1alpha1.Reconciled,
Expand All @@ -272,7 +274,7 @@ func (r KeplerInternalReconciler) updateReconciledStatus(ctx context.Context, ki
reconciled.Message = recErr.Error()
}

return updateCondition(ki.Status.Exporter.Conditions, reconciled)
return updateCondition(ki.Status.Exporter.Conditions, reconciled, time)
}

func findCondition(conditions []v1alpha1.Condition, t v1alpha1.ConditionType) *v1alpha1.Condition {
Expand All @@ -285,7 +287,7 @@ func findCondition(conditions []v1alpha1.Condition, t v1alpha1.ConditionType) *v
}

// returns true if the condition has been updated
func updateCondition(conditions []v1alpha1.Condition, latest v1alpha1.Condition) bool {
func updateCondition(conditions []v1alpha1.Condition, latest v1alpha1.Condition, time metav1.Time) bool {

old := findCondition(conditions, latest.Type)
if old == nil {
Expand All @@ -303,15 +305,17 @@ func updateCondition(conditions []v1alpha1.Condition, latest v1alpha1.Condition)
old.Status = latest.Status
old.Reason = latest.Reason
old.Message = latest.Message
// NOTE: last transition time changes only if the status changes
old.LastTransitionTime = time
return true
}

func (r KeplerInternalReconciler) updateAvailableStatus(ctx context.Context, ki *v1alpha1.KeplerInternal, recErr error) bool {
func (r KeplerInternalReconciler) updateAvailableStatus(ctx context.Context, ki *v1alpha1.KeplerInternal, recErr error, time metav1.Time) bool {
// get daemonset owned by kepler
dset := appsv1.DaemonSet{}
key := types.NamespacedName{Name: ki.DaemonsetName(), Namespace: ki.Namespace()}
if err := r.Client.Get(ctx, key, &dset); err != nil {
return updateCondition(ki.Status.Exporter.Conditions, availableConditionForGetError(err))
return updateCondition(ki.Status.Exporter.Conditions, availableConditionForGetError(err), time)
}

ds := dset.Status
Expand All @@ -323,14 +327,18 @@ func (r KeplerInternalReconciler) updateAvailableStatus(ctx context.Context, ki
ki.Status.Exporter.NumberAvailable = ds.NumberAvailable
ki.Status.Exporter.NumberUnavailable = ds.NumberUnavailable

c := availableCondition(&dset)
available := availableCondition(&dset)

if recErr == nil {
c.ObservedGeneration = ki.Generation
available.ObservedGeneration = ki.Generation
} else {
// failure to reconcile is a Degraded condition
available.Status = v1alpha1.ConditionDegraded
available.Reason = v1alpha1.ReconcileError
}

updated := updateCondition(ki.Status.Exporter.Conditions, c)
updated := updateCondition(ki.Status.Exporter.Conditions, available, time)

// update estimator status
estimatorStatus := v1alpha1.EstimatorStatus{
Status: v1alpha1.DeploymentNotInstalled,
}
Expand Down
7 changes: 4 additions & 3 deletions tests/e2e/kepler_internal_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,12 +257,13 @@ func TestKeplerInternal_WithEstimatorAndModelServer(t *testing.T) {
containers := ds.Spec.Template.Spec.Containers
// deamonset must have a sidecar
assert.Equal(t, 2, len(containers))
// test expected status
f.AssertInternalStatus(ki.Name, test.Timeout(5*time.Minute))

// confirm model-server deployment ready
deploy := appsv1.Deployment{}
f.AssertResourceExists(ki.ModelServerDeploymentName(), testNs, &deploy)
f.AssertResourceExists(ki.ModelServerDeploymentName(), testNs, &deploy, test.Timeout(5*time.Minute))
readyReplicas := deploy.Status.ReadyReplicas
assert.Equal(t, int32(1), readyReplicas)

// test expected status
f.AssertInternalStatus(ki.Name, test.Timeout(2*time.Minute))
}
Loading