-
Notifications
You must be signed in to change notification settings - Fork 44
Reconcile resources #51
Reconcile resources #51
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
return err | ||
} | ||
helper := resource.NewHelper(expected.Client, expected.Mapping) | ||
_, err = helper.Create(expected.Namespace, true, expected.Object) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is interesting. I've never come across the cli-runtime or Info before. Quite convenient that the tiller kubeclient can provide that for us.
// - If the custom resource does not have a release, a new release will be installed | ||
// - If the custom resource has changes for an existing release, the release will be updated | ||
// - If the custom resource has no changes for an existing release, the underlying resources will be reconciled. | ||
func (c installer) ReconcileRelease(r *unstructured.Unstructured) (*unstructured.Unstructured, bool, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One more thing. Not that I can see this anywhere but you might want to double check that none of the installer methods are modifying the installer. Otherwise we should use pointer receivers (c *installer)
. If not then this is fine.
Just pointing it out since its been a while since I've seen non pointer receivers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The struct fields are storageBackend
, tillerKubeClient
and chartDir
, none of which should change, so I think non-pointer receivers is what we want.
This PR add several improvements. It: