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

Unstructured support in different functions #112

Merged
merged 6 commits into from
Aug 10, 2020
Merged

Unstructured support in different functions #112

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
24073d9
Handles both unstructured and commonObject
somtochiama Jul 22, 2020
9f56540
Fixes unstructured for patches and application transform
somtochiama Jul 22, 2020
c3a2e08
Removes kstatus
somtochiama Jul 23, 2020
5ae7327
Moved the type-check around
somtochiama Jul 31, 2020
1a49843
Formats go code
somtochiama Jul 31, 2020
9e95327
Adds formatting directive for Errorf
somtochiama Jul 31, 2020
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
26 changes: 22 additions & 4 deletions pkg/patterns/addon/application.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"errors"
"fmt"

"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
addonsv1alpha1 "sigs.k8s.io/kubebuilder-declarative-pattern/pkg/patterns/addon/pkg/apis/v1alpha1"
"sigs.k8s.io/kubebuilder-declarative-pattern/pkg/patterns/declarative"
"sigs.k8s.io/kubebuilder-declarative-pattern/pkg/patterns/declarative/pkg/manifest"
Expand All @@ -42,8 +43,25 @@ const (

// TransformApplicationFromStatus modifies the Application in the deployment based off the CommonStatus
func TransformApplicationFromStatus(ctx context.Context, instance declarative.DeclarativeObject, objects *manifest.Objects) error {
addonObject, ok := instance.(addonsv1alpha1.CommonObject)
if !ok {
var version string
var healthy bool
var addonObject addonsv1alpha1.CommonObject

if unstruct, ok := instance.(*unstructured.Unstructured); ok {
Copy link
Contributor

Choose a reason for hiding this comment

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

Looking at the other PRs, we might want to create helper functions here. e.g. GetVersion and GetHealthy (or maybe GetCommonSpec and GetCommonStatus)

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 could definitely work on this. I think it is a good idea

v, _, err := unstructured.NestedString(unstruct.Object, "spec", "version")
if err != nil {
return fmt.Errorf("unable to get version from unstuctured: %v", err)
}
version = v

healthy, _, err = unstructured.NestedBool(unstruct.Object, "status", "healthy")
if err != nil {
return fmt.Errorf("unable to get status from unstuctured: %v", err)
}
} else if addonObject, ok = instance.(addonsv1alpha1.CommonObject); ok {
version = addonObject.CommonSpec().Version
healthy = addonObject.GetCommonStatus().Healthy
} else {
return fmt.Errorf("instance %T was not an addonsv1alpha1.CommonObject", instance)
}

Expand All @@ -56,12 +74,12 @@ func TransformApplicationFromStatus(ctx context.Context, instance declarative.De
}

assemblyPhase := Pending
if addonObject.GetCommonStatus().Healthy {
if healthy {
assemblyPhase = Succeeded
}

// TODO: Version should be on CommonStatus as well
app.SetNestedField(addonObject.CommonSpec().Version, "spec", "descriptor", "version")
app.SetNestedField(version, "spec", "descriptor", "version")
app.SetNestedField(assemblyPhase, "spec", "assemblyPhase")

return nil
Expand Down
40 changes: 26 additions & 14 deletions pkg/patterns/addon/patch.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,25 +35,37 @@ import (
func ApplyPatches(ctx context.Context, object declarative.DeclarativeObject, objects *manifest.Objects) error {
log := log.Log

p, ok := object.(addonsv1alpha1.Patchable)
if !ok {
return fmt.Errorf("provided object (%T) does not implement Patchable type", object)
}

var patches []*unstructured.Unstructured

for _, p := range p.PatchSpec().Patches {
// Object is nil, Raw is populated (with json, even when input was yaml)
r := bytes.NewReader(p.Raw)
decoder := yaml.NewYAMLOrJSONDecoder(r, 1024)
patch := &unstructured.Unstructured{}
unstruct, ok := object.(*unstructured.Unstructured)
if ok {
patch, _, err := unstructured.NestedSlice(unstruct.Object, "spec", "patches")
if err != nil {
return fmt.Errorf("unable to get patches from unstructured: %v", err)
}

if err := decoder.Decode(patch); err != nil {
return fmt.Errorf("error parsing json into unstructured object: %v", err)
for _, p := range patch {
m := p.(map[string]interface{})
patches = append(patches, &unstructured.Unstructured{
Object: m,
})
}
log.WithValues("patch", patch).V(1).Info("parsed patch")
} else if p, ok := object.(addonsv1alpha1.Patchable); ok {
for _, p := range p.PatchSpec().Patches {
// Object is nil, Raw is populated (with json, even when input was yaml)
r := bytes.NewReader(p.Raw)
decoder := yaml.NewYAMLOrJSONDecoder(r, 1024)
patch := &unstructured.Unstructured{}

patches = append(patches, patch)
if err := decoder.Decode(patch); err != nil {
return fmt.Errorf("error parsing json into unstructured object: %v", err)
}
log.WithValues("patch", patch).V(1).Info("parsed patch")

patches = append(patches, patch)
}
} else {
return fmt.Errorf("provided object (%T) does not implement Patchable type", object)
}

return objects.Patch(patches)
Expand Down
37 changes: 29 additions & 8 deletions pkg/patterns/addon/pkg/loaders/fs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"flag"
"fmt"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"strings"

"k8s.io/apimachinery/pkg/runtime"
Expand Down Expand Up @@ -53,16 +54,36 @@ func NewManifestLoader(channel string) (*ManifestLoader, error) {
func (c *ManifestLoader) ResolveManifest(ctx context.Context, object runtime.Object) (map[string]string, error) {
log := log.Log

addonObject, ok := object.(addonsv1alpha1.CommonObject)
if !ok {
return nil, fmt.Errorf("object %T was not an addonsv1alpha1.CommonObject", object)
}
var (
channelName string
version string
componentName string
)

componentName := addonObject.ComponentName()
unstruct, ok := object.(*unstructured.Unstructured)
if ok {
v, _, err := unstructured.NestedString(unstruct.Object, "spec", "version")
if err != nil {
return nil, fmt.Errorf("unable to get spec.version: %v", err)
}
version = v

spec := addonObject.CommonSpec()
version := spec.Version
channelName := spec.Channel
c, _, err := unstructured.NestedString(unstruct.Object, "spec", "channel")
if err != nil {
return nil, fmt.Errorf("unable to get spec.version: %v", err)
}
channelName = c

componentName = strings.ToLower(unstruct.GetKind())
} else if addonObject, ok := object.(addonsv1alpha1.CommonObject); ok {
componentName = addonObject.ComponentName()

spec := addonObject.CommonSpec()
version = spec.Version
channelName = spec.Channel
} else {
return nil, fmt.Errorf("object %T was not an addonsv1alpha1.CommonObject", object)
}

// TODO: We should actually do id (1.1.2-aws or 1.1.1-nginx). But maybe YAGNI
id := version
Expand Down
83 changes: 64 additions & 19 deletions pkg/patterns/addon/pkg/status/aggregate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package status
import (
"context"
"fmt"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"reflect"

appsv1 "k8s.io/api/apps/v1"
Expand Down Expand Up @@ -46,50 +47,94 @@ type aggregator struct {
func (a *aggregator) Reconciled(ctx context.Context, src declarative.DeclarativeObject, objs *manifest.Objects) error {
log := log.Log

instance, ok := src.(addonv1alpha1.CommonObject)
if !ok {
return fmt.Errorf("object %T was not an addonv1alpha1.CommonObject", src)
}

status := addonv1alpha1.CommonStatus{Healthy: true}
statusHealthy := true
statusErrors := []string{}

for _, o := range objs.Items {
gk := o.Group + "/" + o.Kind
healthy := true
var err error
switch gk {
case "/Service":
healthy, err = a.service(ctx, instance, o.Name)
healthy, err = a.service(ctx, src, o.Name)
case "extensions/Deployment", "apps/Deployment":
healthy, err = a.deployment(ctx, instance, o.Name)
healthy, err = a.deployment(ctx, src, o.Name)
default:
log.WithValues("type", gk).V(2).Info("type not implemented for status aggregation, skipping")
}

status.Healthy = status.Healthy && healthy
statusHealthy = statusHealthy && healthy
if err != nil {
status.Errors = append(status.Errors, fmt.Sprintf("%v", err))
statusErrors = append(statusErrors, fmt.Sprintf("%v", err))
}
}

log.WithValues("object", src).WithValues("status", status).V(2).Info("built status")
log.WithValues("object", src).WithValues("status", statusHealthy).V(2).Info("built status")

if !reflect.DeepEqual(status, instance.GetCommonStatus()) {
instance.SetCommonStatus(status)
unstruct, ok := src.(*unstructured.Unstructured)
instance, commonOkay := src.(addonv1alpha1.CommonObject)

log.WithValues("name", instance.GetName()).WithValues("status", status).Info("updating status")
unstructStatus := make(map[string]interface{})
var status addonv1alpha1.CommonStatus

if ok {
unstructStatus["Healthy"] = true
} else if commonOkay {
status = addonv1alpha1.CommonStatus{Healthy: true}
} else {
return fmt.Errorf("object %T was not an addonv1alpha1.CommonObject", src)
}

err := a.client.Update(ctx, instance)
if commonOkay {
status.Errors = statusErrors
status.Healthy = statusHealthy

if !reflect.DeepEqual(status, instance.GetCommonStatus()) {
instance.SetCommonStatus(status)

log.WithValues("name", instance.GetName()).WithValues("status", status).Info("updating status")

err := a.client.Update(ctx, instance)
if err != nil {
log.Error(err, "updating status")
return err
}
}
} else {
unstructStatus["Healthy"] = true
unstructStatus["Errors"] = statusErrors
s, _, err := unstructured.NestedMap(unstruct.Object, "status")
if err != nil {
log.Error(err, "updating status")
return err
log.Error(err, "getting status")
return fmt.Errorf("unable to get status from unstructured: %v", err)
}
if !reflect.DeepEqual(status, s) {
err = unstructured.SetNestedField(unstruct.Object, statusHealthy, "status", "healthy")
Copy link
Contributor

Choose a reason for hiding this comment

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

Missed error-check here

if err != nil {
log.Error(err, "updating status")
return fmt.Errorf("unable to set status in unstructured: %v", err)
}

err = unstructured.SetNestedStringSlice(unstruct.Object, statusErrors, "status", "errors")
if err != nil {
log.Error(err, "updating status")
return fmt.Errorf("unable to set status in unstructured: %v", err)
}

log.WithValues("name", unstruct.GetName()).WithValues("status", status).Info("updating status")

err = a.client.Update(ctx, unstruct)
Copy link
Contributor

Choose a reason for hiding this comment

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

If you make this a.client.Update(ctx, src), can you eliminate the duplication between the two if branches?

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 don't know if this would work as the status wasn't changed on src. Unlike the SetNestedField in unstruct and SetCommonStatus in instance that changes the status

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we know where we don't change src? src and unstruct are just a pointer-cast, so they should be the same object.

(Also, if we're changing status we might have to use client.Status().Update(...)

Copy link
Member Author

Choose a reason for hiding this comment

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

It seems every time we run the update function, src has changed. I will change to client.Status().Update(...) in another PR

if err != nil {
log.Error(err, "updating status")
return err
}
}
}

return nil
}

func (a *aggregator) deployment(ctx context.Context, src addonv1alpha1.CommonObject, name string) (bool, error) {
func (a *aggregator) deployment(ctx context.Context, src declarative.DeclarativeObject, name string) (bool, error) {
key := client.ObjectKey{Namespace: src.GetNamespace(), Name: name}
dep := &appsv1.Deployment{}

Expand All @@ -106,7 +151,7 @@ func (a *aggregator) deployment(ctx context.Context, src addonv1alpha1.CommonObj
return false, fmt.Errorf("deployment (%s) does not meet condition: %s", key, successfulDeployment)
}

func (a *aggregator) service(ctx context.Context, src addonv1alpha1.CommonObject, name string) (bool, error) {
func (a *aggregator) service(ctx context.Context, src declarative.DeclarativeObject, name string) (bool, error) {
key := client.ObjectKey{Namespace: src.GetNamespace(), Name: name}
svc := &corev1.Service{}
err := a.client.Get(ctx, key, svc)
Expand Down