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

No panic when getting a plan status for an instance with no plan #461

Merged
merged 3 commits into from
Jul 1, 2019
Merged

No panic when getting a plan status for an instance with no plan #461

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
85d9a9c
No panic when getting a plan status for an instance with no plan
zen-dog Jun 28, 2019
3bac6f7
Merge branch 'master' into ad/issues/459
kensipe Jun 28, 2019
7417223
Merge branch 'master' into ad/issues/459
alenkacz Jul 1, 2019
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 go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ require (
github.com/go-playground/universal-translator v0.16.0 // indirect
github.com/go-test/deep v1.0.1
github.com/gobuffalo/envy v1.6.15 // indirect
github.com/gogo/protobuf v1.2.0 // indirect
github.com/gogo/protobuf v1.2.0
github.com/golang/groupcache v0.0.0-20190129154638-5b532d6fd5ef // indirect
github.com/golang/protobuf v1.3.1 // indirect
github.com/google/go-github v17.0.0+incompatible
Expand All @@ -41,7 +41,7 @@ require (
github.com/prometheus/procfs v0.0.0-20190209105433-f8d8b3f739bd // indirect
github.com/rogpeppe/go-internal v1.2.2 // indirect
github.com/spf13/cobra v0.0.3
github.com/spf13/pflag v1.0.3 // indirect
github.com/spf13/pflag v1.0.3
github.com/stretchr/testify v1.3.0
github.com/xlab/treeprint v0.0.0-20181112141820-a009c3971eca
golang.org/x/crypto v0.0.0-20190424203555-c05e17bb3b2d // indirect
Expand All @@ -62,7 +62,8 @@ require (
k8s.io/apimachinery v0.0.0-20190404173353-6a84e37a896d
k8s.io/client-go v11.0.1-0.20190409021438-1a26190bd76a+incompatible
k8s.io/code-generator v0.0.0-20181117043124-c2090bec4d9b
k8s.io/gengo v0.0.0-20190128074634-0689ccc1d7d6 // indirect
k8s.io/gengo v0.0.0-20190128074634-0689ccc1d7d6
k8s.io/klog v0.3.0
k8s.io/kube-openapi v0.0.0-20190208205540-d7c86cdc46e3 // indirect
sigs.k8s.io/controller-runtime v0.2.0-beta.1.0.20190619203651-3bc157084f53
sigs.k8s.io/controller-tools v0.1.11
Expand Down
14 changes: 10 additions & 4 deletions pkg/controller/planexecution/planexecution_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,12 @@ package planexecution

import (
"context"
"encoding/json"
"fmt"
"log"
"strconv"

"k8s.io/apimachinery/pkg/util/json"
apijson "k8s.io/apimachinery/pkg/util/json"

kudoengine "github.com/kudobuilder/kudo/pkg/engine"

Expand Down Expand Up @@ -489,11 +490,11 @@ func (r *ReconcilePlanExecution) Reconcile(request reconcile.Request) (reconcile
key, _ := client.ObjectKeyFromObject(obj)
truth := obj.DeepCopyObject()
err := r.Client.Get(context.TODO(), key, truth)
rawObj, _ := json.Marshal(obj)
rawObj, _ := apijson.Marshal(obj)
if err == nil {
log.Printf("PlanExecutionController: CreateOrUpdate Object present")
//update
log.Printf("Going to apply patch\n%+v\n\n to object\n%+v\n", string(rawObj), truth)
log.Printf("Going to apply patch\n%+v\n\n to object\n%s\n", string(rawObj), prettyPrint(truth))
if err != nil {
log.Printf("Error getting patch between truth and obj: %v\n", err)
} else {
Expand Down Expand Up @@ -525,7 +526,7 @@ func (r *ReconcilePlanExecution) Reconcile(request reconcile.Request) (reconcile
}
err = health.IsHealthy(r.Client, obj)
if err != nil {
log.Printf("PlanExecutionController: Obj is NOT healthy: %+v", obj)
log.Printf("PlanExecutionController: Obj is NOT healthy: %s", prettyPrint(obj))
planExecution.Status.Phases[i].Steps[j].State = kudov1alpha1.PhaseStateInProgress
planExecution.Status.Phases[i].State = kudov1alpha1.PhaseStateInProgress
}
Expand Down Expand Up @@ -621,3 +622,8 @@ func (r *ReconcilePlanExecution) Cleanup(obj runtime.Object) error {

return nil
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is unrelated to the bug, just making our lives easier when debugging integration tests.

func prettyPrint(i interface{}) string {
s, _ := json.MarshalIndent(i, "", " ")
return string(s)
}
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't this be in a json util package? this is not planexec specific

Copy link
Member

Choose a reason for hiding this comment

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

as I reason through this more... this function literal swallows the err. what is the output for an error? do we not want any indication there was an error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's why I haven't put it into util package - I didn't want a general purpose prettyPrint method for everybody (though it can be). So far, I only saw planexecution_controller printing gigantic yaml object in our logs.

5 changes: 5 additions & 0 deletions pkg/kudoctl/cmd/plan/plan_status.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package plan
import (
"encoding/json"
"fmt"
"log"

kudov1alpha1 "github.com/kudobuilder/kudo/pkg/apis/kudo/v1alpha1"
"github.com/kudobuilder/kudo/pkg/kudoctl/util/check"
Expand Down Expand Up @@ -142,6 +143,10 @@ func planStatus(options *statusOptions) error {
Resource: "planexecutions",
}

if instance.Status.ActivePlan.Name == "" {
log.Printf("No active plan exists for instance %s", instance.Name)
return nil
}
activePlanObj, err := dynamicClient.Resource(planExecutionsGVR).Namespace(options.namespace).Get(instance.Status.ActivePlan.Name, metav1.GetOptions{})
if err != nil {
return err
Expand Down