Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
34 changes: 34 additions & 0 deletions pkg/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -544,6 +544,30 @@ func (po *SubResourcePatchOptions) ApplyToSubResourcePatch(o *SubResourcePatchOp
}
}

// SubResourceApplyOptions are the options for a subresource
// apply rquest.
type SubResourceApplyOptions struct {
ApplyOptions
SubResourceBody runtime.ApplyConfiguration
}

// ApplyOpts applies the given options.
func (ao *SubResourceApplyOptions) ApplyOpts(opts []SubResourceApplyOption) *SubResourceApplyOptions {
Copy link
Member Author

Choose a reason for hiding this comment

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

This differs from our typical naming convention of naming these ApplyOptions because that name is already used for the embedded field. I chose to embedd the field, because there is prior art of doing that in the SubResourcePatchOptions and it seems ergonomical to do it that way, because modifying options doesn't requier to know whether they are defined in ApplyOptions or SubResourceApplyOptions.

for _, o := range opts {
o.ApplyToSubResourceApply(ao)
}

return ao
}

// ApplyToSubResourceApply applies the configuration on the given patch options.
func (ao *SubResourceApplyOptions) ApplyToSubResourceApply(o *SubResourceApplyOptions) {
ao.ApplyOptions.ApplyToApply(&o.ApplyOptions)
if ao.SubResourceBody != nil {
o.SubResourceBody = ao.SubResourceBody
}
}

func (sc *subResourceClient) Get(ctx context.Context, obj Object, subResource Object, opts ...SubResourceGetOption) error {
switch obj.(type) {
case runtime.Unstructured:
Expand Down Expand Up @@ -595,3 +619,13 @@ func (sc *subResourceClient) Patch(ctx context.Context, obj Object, patch Patch,
return sc.client.typedClient.PatchSubResource(ctx, obj, sc.subResource, patch, opts...)
}
}

func (sc *subResourceClient) Apply(ctx context.Context, obj runtime.ApplyConfiguration, opts ...SubResourceApplyOption) error {
switch obj := obj.(type) {
case *unstructuredApplyConfiguration:
defer sc.client.resetGroupVersionKind(obj, obj.GetObjectKind().GroupVersionKind())
return sc.client.unstructuredClient.ApplySubResource(ctx, obj, sc.subResource, opts...)
default:
return sc.client.typedClient.ApplySubResource(ctx, obj, sc.subResource, opts...)
}
}
120 changes: 118 additions & 2 deletions pkg/client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ import (
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
appsv1applyconfigurations "k8s.io/client-go/applyconfigurations/apps/v1"
autoscaling1applyconfigurations "k8s.io/client-go/applyconfigurations/autoscaling/v1"
corev1applyconfigurations "k8s.io/client-go/applyconfigurations/core/v1"
kscheme "k8s.io/client-go/kubernetes/scheme"
"k8s.io/client-go/rest"
Expand Down Expand Up @@ -1127,6 +1129,34 @@ U5wwSivyi7vmegHKmblOzNVKA5qPO8zWzqBC
Expect(err).NotTo(HaveOccurred())
Expect(*dep.Spec.Replicas).To(Equal(replicaCount))
})

It("should be able to apply the scale subresource", func(ctx SpecContext) {
cl, err := client.New(cfg, client.Options{})
Expect(err).NotTo(HaveOccurred())
Expect(cl).NotTo(BeNil())

By("Creating a deployment")
dep, err := clientset.AppsV1().Deployments(dep.Namespace).Create(ctx, dep, metav1.CreateOptions{})
Expect(err).NotTo(HaveOccurred())
replicaCount := *dep.Spec.Replicas + 1

By("Applying the scale subresurce")
deploymentAC, err := appsv1applyconfigurations.ExtractDeployment(dep, "foo")
Expect(err).NotTo(HaveOccurred())
scale := autoscaling1applyconfigurations.Scale().
WithSpec(autoscaling1applyconfigurations.ScaleSpec().WithReplicas(replicaCount))
err = cl.SubResource("scale").Apply(ctx, deploymentAC,
&client.SubResourceApplyOptions{SubResourceBody: scale},
client.FieldOwner("foo"),
client.ForceOwnership,
)
Expect(err).NotTo(HaveOccurred())

By("Asserting replicas got updated")
dep, err = clientset.AppsV1().Deployments(dep.Namespace).Get(ctx, dep.Name, metav1.GetOptions{})
Expect(err).NotTo(HaveOccurred())
Expect(*dep.Spec.Replicas).To(Equal(replicaCount))
})
})

Context("with unstructured objects", func() {
Expand Down Expand Up @@ -1322,8 +1352,8 @@ U5wwSivyi7vmegHKmblOzNVKA5qPO8zWzqBC
By("Creating a deployment")
dep, err := clientset.AppsV1().Deployments(dep.Namespace).Create(ctx, dep, metav1.CreateOptions{})
Expect(err).NotTo(HaveOccurred())
dep.APIVersion = "apps/v1"
dep.Kind = "Deployment"
dep.APIVersion = appsv1.SchemeGroupVersion.String()
dep.Kind = "Deployment" //nolint:goconst
depUnstructured, err := toUnstructured(dep)
Expect(err).NotTo(HaveOccurred())

Expand Down Expand Up @@ -1374,6 +1404,41 @@ U5wwSivyi7vmegHKmblOzNVKA5qPO8zWzqBC
Expect(err).NotTo(HaveOccurred())
Expect(*dep.Spec.Replicas).To(Equal(replicaCount))
})

It("should be able to apply the scale subresource", func(ctx SpecContext) {
cl, err := client.New(cfg, client.Options{Scheme: runtime.NewScheme()})
Expect(err).NotTo(HaveOccurred())
Expect(cl).NotTo(BeNil())

By("Creating a deployment")
dep, err := clientset.AppsV1().Deployments(dep.Namespace).Create(ctx, dep, metav1.CreateOptions{})
Expect(err).NotTo(HaveOccurred())
dep.APIVersion = "apps/v1"
dep.Kind = "Deployment"
depUnstructured, err := toUnstructured(dep)
Expect(err).NotTo(HaveOccurred())

By("Updating the scale subresurce")
replicaCount := *dep.Spec.Replicas + 1
scale := &unstructured.Unstructured{}
scale.SetAPIVersion("autoscaling/v1")
scale.SetKind("Scale")
Expect(unstructured.SetNestedField(scale.Object, int64(replicaCount), "spec", "replicas")).NotTo(HaveOccurred())
err = cl.SubResource("scale").Apply(ctx,
client.ApplyConfigurationFromUnstructured(depUnstructured),
&client.SubResourceApplyOptions{SubResourceBody: client.ApplyConfigurationFromUnstructured(scale)},
client.FieldOwner("foo"),
client.ForceOwnership,
)
Expect(err).NotTo(HaveOccurred())
Expect(scale.GetAPIVersion()).To(Equal("autoscaling/v1"))
Expect(scale.GetKind()).To(Equal("Scale"))

By("Asserting replicas got updated")
dep, err = clientset.AppsV1().Deployments(dep.Namespace).Get(ctx, dep.Name, metav1.GetOptions{})
Expect(err).NotTo(HaveOccurred())
Expect(*dep.Spec.Replicas).To(Equal(replicaCount))
})
})

})
Expand Down Expand Up @@ -1440,6 +1505,29 @@ U5wwSivyi7vmegHKmblOzNVKA5qPO8zWzqBC
Expect(dep.GroupVersionKind()).To(Equal(depGvk))
})

It("should apply status", func(ctx SpecContext) {
cl, err := client.New(cfg, client.Options{})
Expect(err).NotTo(HaveOccurred())
Expect(cl).NotTo(BeNil())

By("initially creating a Deployment")
dep, err := clientset.AppsV1().Deployments(ns).Create(ctx, dep, metav1.CreateOptions{})
Expect(err).NotTo(HaveOccurred())
Expect(dep.Status.Replicas).To(BeEquivalentTo(0))

By("applying the status of Deployment")
deploymentAC, err := appsv1applyconfigurations.ExtractDeployment(dep, "foo")
Expect(err).NotTo(HaveOccurred())
deploymentAC.WithStatus(&appsv1applyconfigurations.DeploymentStatusApplyConfiguration{
Replicas: ptr.To(int32(1)),
})
Expect(cl.Status().Apply(ctx, deploymentAC, client.FieldOwner("foo"))).To(Succeed())

dep, err = clientset.AppsV1().Deployments(ns).Get(ctx, dep.Name, metav1.GetOptions{})
Expect(err).NotTo(HaveOccurred())
Expect(dep.Status.Replicas).To(BeEquivalentTo(1))
})

It("should not update spec of an existing object", func(ctx SpecContext) {
cl, err := client.New(cfg, client.Options{})
Expect(err).NotTo(HaveOccurred())
Expand Down Expand Up @@ -1592,6 +1680,34 @@ U5wwSivyi7vmegHKmblOzNVKA5qPO8zWzqBC
Expect(actual.Status.Replicas).To(BeEquivalentTo(1))
})

It("should apply status and preserve type information", func(ctx SpecContext) {
cl, err := client.New(cfg, client.Options{})
Expect(err).NotTo(HaveOccurred())
Expect(cl).NotTo(BeNil())

By("initially creating a Deployment")
dep, err := clientset.AppsV1().Deployments(ns).Create(ctx, dep, metav1.CreateOptions{})
Expect(err).NotTo(HaveOccurred())
Expect(dep.Status.Replicas).To(BeEquivalentTo(0))

By("applying the status of Deployment")
dep.Status.Replicas = 1
dep.ManagedFields = nil // Must be unset in SSA requests
u := &unstructured.Unstructured{}
Expect(scheme.Convert(dep, u, nil)).To(Succeed())
err = cl.Status().Apply(ctx, client.ApplyConfigurationFromUnstructured(u), client.FieldOwner("foo"))
Expect(err).NotTo(HaveOccurred())

By("validating updated Deployment has type information")
Expect(u.GroupVersionKind()).To(Equal(depGvk))

By("validating patched Deployment has new status")
actual, err := clientset.AppsV1().Deployments(ns).Get(ctx, dep.Name, metav1.GetOptions{})
Expect(err).NotTo(HaveOccurred())
Expect(actual).NotTo(BeNil())
Expect(actual.Status.Replicas).To(BeEquivalentTo(1))
})

It("should not update spec of an existing object", func(ctx SpecContext) {
cl, err := client.New(cfg, client.Options{})
Expect(err).NotTo(HaveOccurred())
Expand Down
4 changes: 4 additions & 0 deletions pkg/client/dryrun.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,3 +132,7 @@ func (sw *dryRunSubResourceClient) Update(ctx context.Context, obj Object, opts
func (sw *dryRunSubResourceClient) Patch(ctx context.Context, obj Object, patch Patch, opts ...SubResourcePatchOption) error {
return sw.client.Patch(ctx, obj, patch, append(opts, DryRunAll)...)
}

func (sw *dryRunSubResourceClient) Apply(ctx context.Context, obj runtime.ApplyConfiguration, opts ...SubResourceApplyOption) error {
return sw.client.Apply(ctx, obj, append(opts, DryRunAll)...)
}
33 changes: 33 additions & 0 deletions pkg/client/dryrun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
appsv1applyconfigurations "k8s.io/client-go/applyconfigurations/apps/v1"
"k8s.io/utils/ptr"

"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -260,4 +261,36 @@ var _ = Describe("DryRunClient", func() {
Expect(actual).NotTo(BeNil())
Expect(actual).To(BeEquivalentTo(dep))
})

It("should not change objects via status apply", func(ctx SpecContext) {
deploymentAC, err := appsv1applyconfigurations.ExtractDeployment(dep, "test-owner")
Expect(err).NotTo(HaveOccurred())
deploymentAC.WithStatus(&appsv1applyconfigurations.DeploymentStatusApplyConfiguration{
Replicas: ptr.To(int32(99)),
})

Expect(getClient().Status().Apply(ctx, deploymentAC, client.FieldOwner("test-owner"))).NotTo(HaveOccurred())

actual, err := clientset.AppsV1().Deployments(ns).Get(ctx, dep.Name, metav1.GetOptions{})
Expect(err).NotTo(HaveOccurred())
Expect(actual).NotTo(BeNil())
Expect(actual).To(BeEquivalentTo(dep))
})

It("should not change objects via status apply with opts", func(ctx SpecContext) {
deploymentAC, err := appsv1applyconfigurations.ExtractDeployment(dep, "test-owner")
Expect(err).NotTo(HaveOccurred())
deploymentAC.WithStatus(&appsv1applyconfigurations.DeploymentStatusApplyConfiguration{
Replicas: ptr.To(int32(99)),
})

opts := &client.SubResourceApplyOptions{ApplyOptions: client.ApplyOptions{DryRun: []string{"Bye", "Pippa"}}}

Expect(getClient().Status().Apply(ctx, deploymentAC, client.FieldOwner("test-owner"), opts)).NotTo(HaveOccurred())

actual, err := clientset.AppsV1().Deployments(ns).Get(ctx, dep.Name, metav1.GetOptions{})
Expect(err).NotTo(HaveOccurred())
Expect(actual).NotTo(BeNil())
Expect(actual).To(BeEquivalentTo(dep))
})
})
32 changes: 32 additions & 0 deletions pkg/client/fake/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -1335,6 +1335,38 @@ func (sw *fakeSubResourceClient) statusPatch(body client.Object, patch client.Pa
return sw.client.patch(body, patch, &patchOptions.PatchOptions)
}

func (sw *fakeSubResourceClient) Apply(ctx context.Context, obj runtime.ApplyConfiguration, opts ...client.SubResourceApplyOption) error {
if sw.subResource != "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.

This is different from Patch, including when using apply through patch which will just patch the main object rather than a subresource when used with subresources other than status.

IMHO it is better to error out rather than to silenly do the wrong thing. We could fo a follow-up to do the same in Patch for consistency (and/or implement the subresources we support through Update there as well)

return errors.New("fakeSubResourceClient currently only supports Apply for status subresource")
}

applyOpts := &client.SubResourceApplyOptions{}
applyOpts.ApplyOpts(opts)

data, err := json.Marshal(obj)
if err != nil {
return fmt.Errorf("failed to marshal apply configuration: %w", err)
}

u := &unstructured.Unstructured{}
if err := json.Unmarshal(data, u); err != nil {
return fmt.Errorf("failed to unmarshal apply configuration: %w", err)
}

patchOpts := &client.SubResourcePatchOptions{}
patchOpts.Raw = applyOpts.AsPatchOptions()

if applyOpts.SubResourceBody != nil {
subResourceBody := &unstructured.Unstructured{}
if err := json.Unmarshal(data, subResourceBody); err != nil {
return fmt.Errorf("failed to unmarshal subresource body: %w", err)
}
patchOpts.SubResourceBody = subResourceBody
}

return sw.Patch(ctx, u, &fakeApplyPatch{}, patchOpts)
}

func allowsUnconditionalUpdate(gvk schema.GroupVersionKind) bool {
switch gvk.Group {
case "apps":
Expand Down
Loading