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

Jump to bottom

pkg/helm: use status conditions, update status for failures #814

Merged
merged 7 commits into from
Dec 17, 2018
Merged

pkg/helm: use status conditions, update status for failures #814

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7140ec6
pkg/helm: use status conditions, update status for failures
joelanford Dec 4, 2018
4b2a6e2
pkg/helm: keep Initialized status condition
joelanford Dec 5, 2018
c9431bc
hack/test/e2e-helm.sh: fixing e2e test
joelanford Dec 6, 2018
5580c03
Merge branch 'master' into helm-status
joelanford Dec 11, 2018
b31e603
pkg/helm/controller/reconcile.go: fix finalizer resource updates
joelanford Dec 11, 2018
acf2202
pkg/predicate,pkg/helm: adding GenerationChangedPredicate
joelanford Dec 6, 2018
ca6dc20
pkg/ansible/controller/controller.go: use GenerationChangedPredicate
joelanford Dec 12, 2018
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
4 changes: 2 additions & 2 deletions hack/tests/e2e-helm.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,13 @@ fi
# create CR
kubectl create -f deploy/crds/helm_v1alpha1_nginx_cr.yaml
trap_add 'kubectl delete --ignore-not-found -f ${DIR2}/deploy/crds/helm_v1alpha1_nginx_cr.yaml' EXIT
if ! timeout 1m bash -c -- 'until kubectl get nginxes.helm.example.com example-nginx -o jsonpath="{..status.release.info.status.code}" | grep 1; do sleep 1; done';
if ! timeout 1m bash -c -- 'until kubectl get nginxes.helm.example.com example-nginx -o jsonpath="{..status.conditions[0].release.info.status.code}" | grep 1; do sleep 1; done';
then
kubectl logs deployment/nginx-operator
exit 1
fi

release_name=$(kubectl get nginxes.helm.example.com example-nginx -o jsonpath="{..status.release.name}")
release_name=$(kubectl get nginxes.helm.example.com example-nginx -o jsonpath="{..status.conditions[0].release.name}")
nginx_deployment=$(kubectl get deployment -l "app.kubernetes.io/instance=${release_name}" -o jsonpath="{..metadata.name}")

if ! timeout 1m kubectl rollout status deployment/${nginx_deployment};
Expand Down
81 changes: 74 additions & 7 deletions pkg/helm/controller/reconcile.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,14 +80,29 @@ func (r HelmOperatorReconciler) Reconcile(request reconcile.Request) (reconcile.
log.V(1).Info("Adding finalizer", "finalizer", finalizer)
finalizers := append(pendingFinalizers, finalizer)
o.SetFinalizers(finalizers)
err := r.Client.Update(context.TODO(), o)
if len(status.Conditions) == 0 {
status.SetCondition(types.HelmAppCondition{
Type: types.ConditionInitializing,
Status: types.StatusTrue,
})
}
err := r.updateResourceStatus(o, status)
return reconcile.Result{}, err
}
status.RemoveCondition(types.ConditionInitializing)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should you keep the initializing condition? (the same way that pod keeps container creating condition) to show that the initialization was completed successfully?


if err := manager.Sync(context.TODO()); err != nil {
log.Error(err, "failed to sync release")
status.SetCondition(types.HelmAppCondition{
Type: types.ConditionIrreconcilable,
Status: types.StatusTrue,
Reason: types.ReasonReconcileError,
Message: err.Error(),
})
_ = r.updateResourceStatus(o, status)
return reconcile.Result{}, err
}
status.RemoveCondition(types.ConditionIrreconcilable)

if deleted {
if !contains(pendingFinalizers, finalizer) {
Expand All @@ -98,15 +113,29 @@ func (r HelmOperatorReconciler) Reconcile(request reconcile.Request) (reconcile.
uninstalledRelease, err := manager.UninstallRelease(context.TODO())
if err != nil && err != release.ErrNotFound {
log.Error(err, "failed to uninstall release")
status.SetCondition(types.HelmAppCondition{
Type: types.ConditionReleaseFailed,
Status: types.StatusTrue,
Reason: types.ReasonUninstallError,
Message: err.Error(),
})
_ = r.updateResourceStatus(o, status)
return reconcile.Result{}, err
}
status.RemoveCondition(types.ConditionReleaseFailed)

if err == release.ErrNotFound {
log.Info("Release not found, removing finalizer")
} else {
log.Info("Uninstalled release")
if log.Enabled() {
fmt.Println(diffutil.Diff(uninstalledRelease.GetManifest(), ""))
}
status.SetCondition(types.HelmAppCondition{
Type: types.ConditionDeployed,
Status: types.StatusFalse,
Reason: types.ReasonUninstallSuccessful,
})
}
finalizers := []string{}
for _, pendingFinalizer := range pendingFinalizers {
Expand All @@ -115,24 +144,38 @@ func (r HelmOperatorReconciler) Reconcile(request reconcile.Request) (reconcile.
}
}
o.SetFinalizers(finalizers)
err = r.Client.Update(context.TODO(), o)
err = r.updateResourceStatus(o, status)
return reconcile.Result{}, err
}

if !manager.IsInstalled() {
installedRelease, err := manager.InstallRelease(context.TODO())
if err != nil {
log.Error(err, "failed to install release")
status.SetCondition(types.HelmAppCondition{
Type: types.ConditionReleaseFailed,
Status: types.StatusTrue,
Reason: types.ReasonInstallError,
Message: err.Error(),
Release: installedRelease,
})
_ = r.updateResourceStatus(o, status)
return reconcile.Result{}, err
}
status.RemoveCondition(types.ConditionReleaseFailed)

log.Info("Installed release")
if log.Enabled() {
fmt.Println(diffutil.Diff("", installedRelease.GetManifest()))
}
log.V(1).Info("Config values", "values", installedRelease.GetConfig())
status.SetRelease(installedRelease)
status.SetPhase(types.PhaseApplied, types.ReasonApplySuccessful, installedRelease.GetInfo().GetStatus().GetNotes())
status.SetCondition(types.HelmAppCondition{
Type: types.ConditionDeployed,
Status: types.StatusTrue,
Reason: types.ReasonInstallSuccessful,
Message: installedRelease.GetInfo().GetStatus().GetNotes(),
Release: installedRelease,
})
err = r.updateResourceStatus(o, status)
return reconcile.Result{RequeueAfter: r.ResyncPeriod}, err
}
Expand All @@ -141,27 +184,51 @@ func (r HelmOperatorReconciler) Reconcile(request reconcile.Request) (reconcile.
previousRelease, updatedRelease, err := manager.UpdateRelease(context.TODO())
if err != nil {
log.Error(err, "failed to update release")
status.SetCondition(types.HelmAppCondition{
Type: types.ConditionReleaseFailed,
Status: types.StatusTrue,
Reason: types.ReasonUpdateError,
Message: err.Error(),
Release: updatedRelease,
})
_ = r.updateResourceStatus(o, status)
return reconcile.Result{}, err
}
status.RemoveCondition(types.ConditionReleaseFailed)

log.Info("Updated release")
if log.Enabled() {
fmt.Println(diffutil.Diff(previousRelease.GetManifest(), updatedRelease.GetManifest()))
}
log.V(1).Info("Config values", "values", updatedRelease.GetConfig())
status.SetRelease(updatedRelease)
status.SetPhase(types.PhaseApplied, types.ReasonApplySuccessful, updatedRelease.GetInfo().GetStatus().GetNotes())
status.SetCondition(types.HelmAppCondition{
Type: types.ConditionDeployed,
Status: types.StatusTrue,
Reason: types.ReasonUpdateSuccessful,
Message: updatedRelease.GetInfo().GetStatus().GetNotes(),
Release: updatedRelease,
})
err = r.updateResourceStatus(o, status)
return reconcile.Result{RequeueAfter: r.ResyncPeriod}, err
}

_, err = manager.ReconcileRelease(context.TODO())
if err != nil {
log.Error(err, "failed to reconcile release")
status.SetCondition(types.HelmAppCondition{
Type: types.ConditionIrreconcilable,
Status: types.StatusTrue,
Reason: types.ReasonReconcileError,
Message: err.Error(),
})
_ = r.updateResourceStatus(o, status)
return reconcile.Result{}, err
}
status.RemoveCondition(types.ConditionIrreconcilable)

log.Info("Reconciled release")
return reconcile.Result{RequeueAfter: r.ResyncPeriod}, nil
err = r.updateResourceStatus(o, status)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just realized this is causing immediate reconciliations even when nothing has changed, so I'll need to come up with a way to prevent this.

Are we updating the CRD scaffold to use the status subresource when we move to 1.12?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joelanford We're trying to do it before 1.12 actually, as it seems CRD subresources are already beta in 1.11 and on by default.
#787 (comment)
https://github.com/operator-framework/operator-sdk/pull/787/files#diff-f44ebe3a96e65181844d62f38ed5a148R60

We can use the status client to only update the status sub resource and have a predicate to filter out status updates by checking metadata.generation.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is getting added here: #787

return reconcile.Result{RequeueAfter: r.ResyncPeriod}, err
}

func (r HelmOperatorReconciler) updateResourceStatus(o *unstructured.Unstructured, status *types.HelmAppStatus) error {
Expand Down
3 changes: 3 additions & 0 deletions pkg/helm/engine/ownerref.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ func (o *OwnerRefEngine) Render(chart *chart.Chart, values chartutil.Values) (ma
ownedRenderedFiles := map[string]string{}
for fileName, renderedFile := range rendered {
if !strings.HasSuffix(fileName, ".yaml") {
// Pass non-YAML files through untouched.
// This is required for NOTES.txt
ownedRenderedFiles[fileName] = renderedFile
continue
}
withOwner, err := o.addOwnerRefs(renderedFile)
Expand Down
101 changes: 63 additions & 38 deletions pkg/helm/internal/types/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,32 +38,41 @@ type HelmApp struct {

type HelmAppSpec map[string]interface{}

type ResourcePhase string
type HelmAppConditionType string
type ConditionStatus string
type HelmAppConditionReason string

const (
PhaseNone ResourcePhase = ""
PhaseApplying ResourcePhase = "Applying"
PhaseApplied ResourcePhase = "Applied"
PhaseFailed ResourcePhase = "Failed"
)
type HelmAppCondition struct {
Type HelmAppConditionType `json:"type"`
Status ConditionStatus `json:"status"`
Reason HelmAppConditionReason `json:"reason,omitempty"`
Message string `json:"message,omitempty"`
Release *release.Release `json:"release,omitempty"`

type ConditionReason string
LastTransitionTime metav1.Time `json:"lastTransitionTime,omitempty"`
}

const (
ReasonUnknown ConditionReason = "Unknown"
ReasonCustomResourceAdded ConditionReason = "CustomResourceAdded"
ReasonCustomResourceUpdated ConditionReason = "CustomResourceUpdated"
ReasonApplySuccessful ConditionReason = "ApplySuccessful"
ReasonApplyFailed ConditionReason = "ApplyFailed"
ConditionInitializing HelmAppConditionType = "Initializing"
ConditionDeployed HelmAppConditionType = "Deployed"
ConditionReleaseFailed HelmAppConditionType = "ReleaseFailed"
ConditionIrreconcilable HelmAppConditionType = "Irreconcilable"

StatusTrue ConditionStatus = "True"
StatusFalse ConditionStatus = "False"
StatusUnknown ConditionStatus = "Unknown"

ReasonInstallSuccessful HelmAppConditionReason = "InstallSuccessful"
ReasonUpdateSuccessful HelmAppConditionReason = "UpdateSuccessful"
ReasonUninstallSuccessful HelmAppConditionReason = "UninstallSuccessful"
ReasonInstallError HelmAppConditionReason = "InstallError"
ReasonUpdateError HelmAppConditionReason = "UpdateError"
ReasonReconcileError HelmAppConditionReason = "ReconcileError"
ReasonUninstallError HelmAppConditionReason = "UninstallError"
)

type HelmAppStatus struct {
Release *release.Release `json:"release"`
Phase ResourcePhase `json:"phase"`
Reason ConditionReason `json:"reason,omitempty"`
Message string `json:"message,omitempty"`
LastUpdateTime metav1.Time `json:"lastUpdateTime,omitempty"`
LastTransitionTime metav1.Time `json:"lastTransitionTime,omitempty"`
Conditions []HelmAppCondition `json:"conditions"`
}

func (s *HelmAppStatus) ToMap() (map[string]interface{}, error) {
Expand All @@ -76,37 +85,53 @@ func (s *HelmAppStatus) ToMap() (map[string]interface{}, error) {
return out, nil
}

// SetPhase takes a custom resource status and returns the updated status, without updating the resource in the cluster.
func (s *HelmAppStatus) SetPhase(phase ResourcePhase, reason ConditionReason, message string) *HelmAppStatus {
s.LastUpdateTime = metav1.Now()
if s.Phase != phase {
s.Phase = phase
s.LastTransitionTime = metav1.Now()
// SetCondition sets a condition on the status object. If the condition already
// exists, it will be replaced. SetCondition does not update the resource in
// the cluster.
func (s *HelmAppStatus) SetCondition(condition HelmAppCondition) *HelmAppStatus {
now := metav1.Now()
for i := range s.Conditions {
if s.Conditions[i].Type == condition.Type {
if s.Conditions[i].Status != condition.Status {
condition.LastTransitionTime = now
} else {
condition.LastTransitionTime = s.Conditions[i].LastTransitionTime
}
s.Conditions[i] = condition
return s
}
}
s.Message = message
s.Reason = reason

// If the condition does not exist,
// initialize the lastTransitionTime
condition.LastTransitionTime = now
s.Conditions = append(s.Conditions, condition)
return s
}

// SetRelease takes a release object and adds or updates the release on the status object
func (s *HelmAppStatus) SetRelease(release *release.Release) *HelmAppStatus {
s.Release = release
// RemoveCondition removes the condition with the passed condition type from
// the status object. If the condition is not already present, the returned
// status object is returned unchanged. RemoveCondition does not update the
// resource in the cluster.
func (s *HelmAppStatus) RemoveCondition(conditionType HelmAppConditionType) *HelmAppStatus {
for i := range s.Conditions {
if s.Conditions[i].Type == conditionType {
s.Conditions = append(s.Conditions[:i], s.Conditions[i+1:]...)
return s
}
}
return s
}

// StatusFor safely returns a typed status block from a custom resource.
func StatusFor(cr *unstructured.Unstructured) *HelmAppStatus {
switch cr.Object["status"].(type) {
switch s := cr.Object["status"].(type) {
case *HelmAppStatus:
return cr.Object["status"].(*HelmAppStatus)
return s
case map[string]interface{}:
var status *HelmAppStatus
if err := runtime.DefaultUnstructuredConverter.FromUnstructured(cr.Object["status"].(map[string]interface{}), &status); err != nil {
return &HelmAppStatus{
Phase: PhaseFailed,
Reason: ReasonApplyFailed,
Message: err.Error(),
}
if err := runtime.DefaultUnstructuredConverter.FromUnstructured(s, &status); err != nil {
return &HelmAppStatus{}
}
return status
default:
Expand Down
Loading