From 2bdc05945d9bea89d939e8c4e253d49db84e0217 Mon Sep 17 00:00:00 2001 From: Karl Isenberg Date: Fri, 4 Mar 2022 17:30:44 -0800 Subject: [PATCH] feat: Add inventory.StatusPolicy - StatusPolicyNone disables inventory status updates. - StatusPolicyAll fully enables inventory status updates. - This allows an opt-out feature for working around the problem that adding status can make the inventory larger than the max etcd object size, causing the applier to exit without applying or pruning anything. With StatusPolicyNone, the user can still safely prune objects to make their inventory smaller, and then re-enable the status with StatusPolicyAll. - Note: the default ConfigMap does not currently support status, so this only affects custom inventory impls. --- pkg/inventory/inventory-client-factory.go | 5 +- pkg/inventory/inventory-client.go | 10 +++- pkg/inventory/inventory-client_test.go | 70 +++++++++++++---------- pkg/inventory/status-policy.go | 18 ++++++ pkg/inventory/statuspolicy_string.go | 24 ++++++++ test/e2e/customprovider/provider.go | 4 +- 6 files changed, 96 insertions(+), 35 deletions(-) create mode 100644 pkg/inventory/status-policy.go create mode 100644 pkg/inventory/statuspolicy_string.go diff --git a/pkg/inventory/inventory-client-factory.go b/pkg/inventory/inventory-client-factory.go index 27092aad..707a6766 100644 --- a/pkg/inventory/inventory-client-factory.go +++ b/pkg/inventory/inventory-client-factory.go @@ -16,8 +16,9 @@ type ClientFactory interface { // ClusterClientFactory is a factory that creates instances of ClusterClient inventory client. type ClusterClientFactory struct { + StatusPolicy StatusPolicy } -func (ClusterClientFactory) NewClient(factory cmdutil.Factory) (Client, error) { - return NewClient(factory, WrapInventoryObj, InvInfoToConfigMap) +func (ccf ClusterClientFactory) NewClient(factory cmdutil.Factory) (Client, error) { + return NewClient(factory, WrapInventoryObj, InvInfoToConfigMap, ccf.StatusPolicy) } diff --git a/pkg/inventory/inventory-client.go b/pkg/inventory/inventory-client.go index d1561d7e..84690701 100644 --- a/pkg/inventory/inventory-client.go +++ b/pkg/inventory/inventory-client.go @@ -54,6 +54,7 @@ type ClusterClient struct { mapper meta.RESTMapper InventoryFactoryFunc StorageFactoryFunc invToUnstructuredFunc ToUnstructuredFunc + statusPolicy StatusPolicy } var _ Client = &ClusterClient{} @@ -62,7 +63,9 @@ var _ Client = &ClusterClient{} // Client interface or an error. func NewClient(factory cmdutil.Factory, invFunc StorageFactoryFunc, - invToUnstructuredFunc ToUnstructuredFunc) (*ClusterClient, error) { + invToUnstructuredFunc ToUnstructuredFunc, + statusPolicy StatusPolicy, +) (*ClusterClient, error) { dc, err := factory.DynamicClient() if err != nil { return nil, err @@ -81,6 +84,7 @@ func NewClient(factory cmdutil.Factory, mapper: mapper, InventoryFactoryFunc: invFunc, invToUnstructuredFunc: invToUnstructuredFunc, + statusPolicy: statusPolicy, } return &clusterClient, nil } @@ -451,6 +455,10 @@ func (cic *ClusterClient) getMapping(obj *unstructured.Unstructured) (*meta.REST } func (cic *ClusterClient) updateStatus(obj *unstructured.Unstructured, dryRun common.DryRunStrategy) error { + if cic.statusPolicy != StatusPolicyAll { + klog.V(4).Infof("inventory status update skipped (StatusPolicy: %s)", cic.statusPolicy) + return nil + } if dryRun.ClientOrServerDryRun() { klog.V(4).Infof("dry-run update inventory status: not updated") return nil diff --git a/pkg/inventory/inventory-client_test.go b/pkg/inventory/inventory-client_test.go index 12226456..f5cc5338 100644 --- a/pkg/inventory/inventory-client_test.go +++ b/pkg/inventory/inventory-client_test.go @@ -39,10 +39,11 @@ func podData(name string) map[string]string { func TestGetClusterInventoryInfo(t *testing.T) { tests := map[string]struct { - inv Info - localObjs object.ObjMetadataSet - objStatus []actuation.ObjectStatus - isError bool + statusPolicy StatusPolicy + inv Info + localObjs object.ObjMetadataSet + objStatus []actuation.ObjectStatus + isError bool }{ "Nil local inventory object is an error": { inv: nil, @@ -83,7 +84,7 @@ func TestGetClusterInventoryInfo(t *testing.T) { for name, tc := range tests { t.Run(name, func(t *testing.T) { invClient, err := NewClient(tf, - WrapInventoryObj, InvInfoToConfigMap) + WrapInventoryObj, InvInfoToConfigMap, tc.statusPolicy) require.NoError(t, err) var inv *unstructured.Unstructured @@ -116,11 +117,12 @@ func TestGetClusterInventoryInfo(t *testing.T) { func TestMerge(t *testing.T) { tests := map[string]struct { - localInv Info - localObjs object.ObjMetadataSet - clusterObjs object.ObjMetadataSet - pruneObjs object.ObjMetadataSet - isError bool + statusPolicy StatusPolicy + localInv Info + localObjs object.ObjMetadataSet + clusterObjs object.ObjMetadataSet + pruneObjs object.ObjMetadataSet + isError bool }{ "Nil local inventory object is error": { localInv: nil, @@ -188,7 +190,7 @@ func TestMerge(t *testing.T) { tf.FakeDynamicClient.PrependReactor("list", "configmaps", toReactionFunc(tc.clusterObjs)) // Create the local inventory object storing "tc.localObjs" invClient, err := NewClient(tf, - WrapInventoryObj, InvInfoToConfigMap) + WrapInventoryObj, InvInfoToConfigMap, tc.statusPolicy) require.NoError(t, err) // Call "Merge" to create the union of clusterObjs and localObjs. @@ -212,10 +214,11 @@ func TestMerge(t *testing.T) { func TestCreateInventory(t *testing.T) { tests := map[string]struct { - inv Info - localObjs object.ObjMetadataSet - error string - objStatus []actuation.ObjectStatus + statusPolicy StatusPolicy + inv Info + localObjs object.ObjMetadataSet + error string + objStatus []actuation.ObjectStatus }{ "Nil local inventory object is an error": { inv: nil, @@ -260,7 +263,7 @@ func TestCreateInventory(t *testing.T) { }) invClient, err := NewClient(tf, - WrapInventoryObj, InvInfoToConfigMap) + WrapInventoryObj, InvInfoToConfigMap, tc.statusPolicy) require.NoError(t, err) inv := invClient.invToUnstructuredFunc(tc.inv) if inv != nil { @@ -288,10 +291,11 @@ func TestCreateInventory(t *testing.T) { func TestReplace(t *testing.T) { tests := map[string]struct { - localObjs object.ObjMetadataSet - clusterObjs object.ObjMetadataSet - objStatus []actuation.ObjectStatus - data map[string]string + statusPolicy StatusPolicy + localObjs object.ObjMetadataSet + clusterObjs object.ObjMetadataSet + objStatus []actuation.ObjectStatus + data map[string]string }{ "Cluster and local inventories empty": { localObjs: object.ObjMetadataSet{}, @@ -336,7 +340,8 @@ func TestReplace(t *testing.T) { defer tf.Cleanup() // Client and server dry-run do not throw errors. - invClient, err := NewClient(tf, WrapInventoryObj, InvInfoToConfigMap) + invClient, err := NewClient(tf, + WrapInventoryObj, InvInfoToConfigMap, StatusPolicyAll) require.NoError(t, err) err = invClient.Replace(copyInventory(), object.ObjMetadataSet{}, nil, common.DryRunClient) if err != nil { @@ -351,7 +356,7 @@ func TestReplace(t *testing.T) { t.Run(name, func(t *testing.T) { // Create inventory client, and store the cluster objs in the inventory object. invClient, err := NewClient(tf, - WrapInventoryObj, InvInfoToConfigMap) + WrapInventoryObj, InvInfoToConfigMap, tc.statusPolicy) require.NoError(t, err) wrappedInv := invClient.InventoryFactoryFunc(inventoryObj) if err := wrappedInv.Store(tc.clusterObjs, tc.objStatus); err != nil { @@ -388,9 +393,10 @@ func TestReplace(t *testing.T) { func TestGetClusterObjs(t *testing.T) { tests := map[string]struct { - localInv Info - clusterObjs object.ObjMetadataSet - isError bool + statusPolicy StatusPolicy + localInv Info + clusterObjs object.ObjMetadataSet + isError bool }{ "Nil cluster inventory is error": { localInv: nil, @@ -421,7 +427,7 @@ func TestGetClusterObjs(t *testing.T) { tf.FakeDynamicClient.PrependReactor("list", "configmaps", toReactionFunc(tc.clusterObjs)) invClient, err := NewClient(tf, - WrapInventoryObj, InvInfoToConfigMap) + WrapInventoryObj, InvInfoToConfigMap, tc.statusPolicy) require.NoError(t, err) clusterObjs, err := invClient.GetClusterObjs(tc.localInv) if tc.isError { @@ -442,9 +448,10 @@ func TestGetClusterObjs(t *testing.T) { func TestDeleteInventoryObj(t *testing.T) { tests := map[string]struct { - inv Info - localObjs object.ObjMetadataSet - objStatus []actuation.ObjectStatus + statusPolicy StatusPolicy + inv Info + localObjs object.ObjMetadataSet + objStatus []actuation.ObjectStatus }{ "Nil local inventory object is an error": { inv: nil, @@ -483,7 +490,7 @@ func TestDeleteInventoryObj(t *testing.T) { defer tf.Cleanup() invClient, err := NewClient(tf, - WrapInventoryObj, InvInfoToConfigMap) + WrapInventoryObj, InvInfoToConfigMap, tc.statusPolicy) require.NoError(t, err) inv := invClient.invToUnstructuredFunc(tc.inv) if inv != nil { @@ -500,6 +507,7 @@ func TestDeleteInventoryObj(t *testing.T) { func TestApplyInventoryNamespace(t *testing.T) { testCases := map[string]struct { + statusPolicy StatusPolicy namespace *unstructured.Unstructured dryRunStrategy common.DryRunStrategy reactorError error @@ -529,7 +537,7 @@ func TestApplyInventoryNamespace(t *testing.T) { }) invClient, err := NewClient(tf, - WrapInventoryObj, InvInfoToConfigMap) + WrapInventoryObj, InvInfoToConfigMap, tc.statusPolicy) require.NoError(t, err) err = invClient.ApplyInventoryNamespace(tc.namespace, tc.dryRunStrategy) assert.NoError(t, err) diff --git a/pkg/inventory/status-policy.go b/pkg/inventory/status-policy.go new file mode 100644 index 00000000..5ce43905 --- /dev/null +++ b/pkg/inventory/status-policy.go @@ -0,0 +1,18 @@ +// Copyright 2022 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 + +package inventory + +// StatusPolicy specifies whether the inventory client should apply status to +// the inventory object. The status contains the actuation and reconcile stauts +// of each object in the inventory. +//go:generate stringer -type=StatusPolicy -linecomment +type StatusPolicy int + +const ( + // StatusPolicyNone disables inventory status updates. + StatusPolicyNone StatusPolicy = iota // None + + // StatusPolicyAll fully enables inventory status updates. + StatusPolicyAll // All +) diff --git a/pkg/inventory/statuspolicy_string.go b/pkg/inventory/statuspolicy_string.go new file mode 100644 index 00000000..6acd1d6f --- /dev/null +++ b/pkg/inventory/statuspolicy_string.go @@ -0,0 +1,24 @@ +// Code generated by "stringer -type=StatusPolicy -linecomment"; DO NOT EDIT. + +package inventory + +import "strconv" + +func _() { + // An "invalid array index" compiler error signifies that the constant values have changed. + // Re-run the stringer command to generate them again. + var x [1]struct{} + _ = x[StatusPolicyNone-0] + _ = x[StatusPolicyAll-1] +} + +const _StatusPolicy_name = "NoneAll" + +var _StatusPolicy_index = [...]uint8{0, 4, 7} + +func (i StatusPolicy) String() string { + if i < 0 || i >= StatusPolicy(len(_StatusPolicy_index)-1) { + return "StatusPolicy(" + strconv.FormatInt(int64(i), 10) + ")" + } + return _StatusPolicy_name[_StatusPolicy_index[i]:_StatusPolicy_index[i+1]] +} diff --git a/test/e2e/customprovider/provider.go b/test/e2e/customprovider/provider.go index 8a428060..ab0642c5 100644 --- a/test/e2e/customprovider/provider.go +++ b/test/e2e/customprovider/provider.go @@ -81,7 +81,9 @@ type CustomClientFactory struct { } func (CustomClientFactory) NewClient(factory util.Factory) (inventory.Client, error) { - return inventory.NewClient(factory, WrapInventoryObj, invToUnstructuredFunc) + // TODO: add status to custom inventory crd and enable StatusPolicyAll + return inventory.NewClient(factory, + WrapInventoryObj, invToUnstructuredFunc, inventory.StatusPolicyNone) } func invToUnstructuredFunc(inv inventory.Info) *unstructured.Unstructured {