From dda31bbf2e7a94624e3679e3db56c95e66509ce0 Mon Sep 17 00:00:00 2001 From: Kevin Delgado Date: Thu, 8 Jul 2021 01:56:58 +0000 Subject: [PATCH] Manually set GVK in extract, add commentary to extractor --- hack_main.go => real_cluster_script.go | 10 +++++-- .../pkg/apis/meta/v1/unstructured/helpers.go | 6 +--- .../pkg/util/managedfields/extract.go | 17 +++++------ .../handlers/fieldmanager/gvkparser.go | 7 ++--- .../meta/v1/unstructured.go | 30 ++++++++++++++----- .../v4/value/reflectcache.go | 1 - 6 files changed, 40 insertions(+), 31 deletions(-) rename hack_main.go => real_cluster_script.go (93%) diff --git a/hack_main.go b/real_cluster_script.go similarity index 93% rename from hack_main.go rename to real_cluster_script.go index 969e69d4cd3..139f9146f2e 100644 --- a/hack_main.go +++ b/real_cluster_script.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "os" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -23,12 +24,17 @@ import ( "sigs.k8s.io/structured-merge-diff/v4/typed" ) +// This is a small hacky script that tests the extract config against a real cluster +// to demonstrate a bug I'm seeing with the GVKParser where it claims there +// are duplicate GVK entries +// TODO: don't commit this, delete it before merging. +// Any and all functionality from this script should be captured in the appropriate integration test. func main() { - fmt.Println("vim-go") defaultNS := "default" mydep := "mydep" mgr := "mymanager" - kubeconfig := "/usr/local/google/home/kevindelgado/.kube/config" + // TODO: make this an arg + kubeconfig := os.Getenv("KUBECONFIG") config, err := clientcmd.BuildConfigFromFlags("", kubeconfig) if err != nil { diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/helpers.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/helpers.go index 05fecca88f3..7b101ea5124 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/helpers.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/helpers.go @@ -337,12 +337,8 @@ func (s unstructuredJSONScheme) Decode(data []byte, _ *schema.GroupVersionKind, } gvk := obj.GetObjectKind().GroupVersionKind() - //goruntime.Breakpoint() if len(gvk.Kind) == 0 { - //goruntime.Breakpoint() - //return nil, &gvk, runtime.NewMissingKindErr(string(data)) - fmt.Println("MISSING KIND, ignoring err") - fmt.Printf("string(data) = %+v\n", string(data)) + return nil, &gvk, runtime.NewMissingKindErr(string(data)) } return obj, &gvk, nil diff --git a/staging/src/k8s.io/apimachinery/pkg/util/managedfields/extract.go b/staging/src/k8s.io/apimachinery/pkg/util/managedfields/extract.go index 7507a25893a..831d6a546dc 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/managedfields/extract.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/managedfields/extract.go @@ -75,16 +75,13 @@ func ExtractInto(object runtime.Object, objectType typed.ParseableType, fieldMan if !ok { return fmt.Errorf("unable to convert managed fields for %s to unstructured, expected map, got %T", fieldManager, u) } - fmt.Printf("u = %+v\n", u) - fmt.Println("---") - fmt.Printf("typedObj = %+v\n", typedObj) - fmt.Println("---") - fmt.Printf("object = %+v\n", object) - fmt.Println("---") - fmt.Printf("m = %+v\n", m) - fmt.Println("---") - fmt.Printf("applyConfiguration = %+v\n", applyConfiguration) - fmt.Println("---") + + // set the type meta manually if it doesn't exist to avoid missing kind errors + // when decoding from unstructured JSON + if _, ok := m["kind"]; !ok { + m["kind"] = object.GetObjectKind().GroupVersionKind().Kind + m["apiVersion"] = object.GetObjectKind().GroupVersionKind().GroupVersion().String() + } if err := runtime.DefaultUnstructuredConverter.FromUnstructured(m, applyConfiguration); err != nil { return fmt.Errorf("error extracting into obj from unstructured: %w", err) } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/gvkparser.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/gvkparser.go index b30c423ab8f..b86a533ac60 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/gvkparser.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/gvkparser.go @@ -65,15 +65,12 @@ func NewGVKParser(models proto.Models, preserveUnknownFields bool) (*GvkParser, panic(fmt.Sprintf("ListModels returns a model that can't be looked-up for: %v", modelName)) } gvkList := parseGroupVersionKind(model) - //fmt.Println("+++") - //fmt.Printf("gvkList = %+v\n", gvkList) - //fmt.Println("+++") for _, gvk := range gvkList { if len(gvk.Kind) > 0 { _, ok := parser.gvks[gvk] if ok { - //return nil, fmt.Errorf("duplicate entry for %v", gvk) - fmt.Printf("duplicate entry for %v\n", gvk) + // TODO: double check why this is failing in real_cluster_script.go + return nil, fmt.Errorf("duplicate entry for %v", gvk) } parser.gvks[gvk] = modelName } diff --git a/staging/src/k8s.io/client-go/applyconfigurations/meta/v1/unstructured.go b/staging/src/k8s.io/client-go/applyconfigurations/meta/v1/unstructured.go index ccecff776e2..043276d142f 100644 --- a/staging/src/k8s.io/client-go/applyconfigurations/meta/v1/unstructured.go +++ b/staging/src/k8s.io/client-go/applyconfigurations/meta/v1/unstructured.go @@ -10,20 +10,27 @@ import ( "sigs.k8s.io/structured-merge-diff/v4/typed" ) +// UnstructuredExtractor enables extracting the applied configuration state from object for fieldManager into an +// unstructured object type. type UnstructuredExtractor interface { ExtractUnstructured(object *unstructured.Unstructured, fieldManager string) (*unstructured.Unstructured, error) ExtractUnstructuredStatus(object *unstructured.Unstructured, fieldManager string) (*unstructured.Unstructured, error) } -type parserCache interface { - parserForGVK(gvk schema.GroupVersionKind) (*typed.ParseableType, error) +// objectTypeCache is a cache of typed.ParseableTypes +type objectTypeCache interface { + objectTypeForGVK(gvk schema.GroupVersionKind) (*typed.ParseableType, error) } -type nonCachingParserCache struct { +// nonCachingObjectTypeCache is a objectTypeCache that does no caching +// (i.e. it downloads the OpenAPISchema every time) +// Useful during the proof-of-concept stage until we agree on a caching solution. +type nonCachingObjectTypeCache struct { discoveryClient *discovery.DiscoveryClient } -func (c *nonCachingParserCache) parserForGVK(gvk schema.GroupVersionKind) (*typed.ParseableType, error) { +// objectTypeForGVK retrieves the typed.ParseableType for a given gvk from the cache +func (c *nonCachingObjectTypeCache) objectTypeForGVK(gvk schema.GroupVersionKind) (*typed.ParseableType, error) { doc, err := c.discoveryClient.OpenAPISchema() if err != nil { return nil, err @@ -43,31 +50,38 @@ func (c *nonCachingParserCache) parserForGVK(gvk schema.GroupVersionKind) (*type } type extractor struct { - cache parserCache + cache objectTypeCache } +// NewUnstructuredExtractor creates the extractor with which you can extract the applied configuration +// for a given manager from an unstructured object. func NewUnstructuredExtractor(dc *discovery.DiscoveryClient) UnstructuredExtractor { return &extractor{ - cache: &nonCachingParserCache{dc}, + cache: &nonCachingObjectTypeCache{dc}, } } +// ExtractUnstructured extracts the applied configuration owned by fiieldManager from an unstructured object. +// Note that the apply configuration itself is also an unstructured object. func (e *extractor) ExtractUnstructured(object *unstructured.Unstructured, fieldManager string) (*unstructured.Unstructured, error) { return e.extractUnstructured(object, fieldManager, "") } +// ExtractUnstructuredStatus is the same as ExtractUnstructured except +// that it extracts the status subresource applied configuration. +// Experimental! func (e *extractor) ExtractUnstructuredStatus(object *unstructured.Unstructured, fieldManager string) (*unstructured.Unstructured, error) { return e.extractUnstructured(object, fieldManager, "status") } func (e *extractor) extractUnstructured(object *unstructured.Unstructured, fieldManager string, subresource string) (*unstructured.Unstructured, error) { gvk := object.GetObjectKind().GroupVersionKind() - parser, err := e.cache.parserForGVK(gvk) + objectType, err := e.cache.objectTypeForGVK(gvk) if err != nil { return nil, err } result := &unstructured.Unstructured{} - err = managedfields.ExtractInto(object, *parser, fieldManager, result, subresource) + err = managedfields.ExtractInto(object, *objectType, fieldManager, result, subresource) if err != nil { return nil, err } diff --git a/vendor/sigs.k8s.io/structured-merge-diff/v4/value/reflectcache.go b/vendor/sigs.k8s.io/structured-merge-diff/v4/value/reflectcache.go index e377ccc5742..a5a467c0f00 100644 --- a/vendor/sigs.k8s.io/structured-merge-diff/v4/value/reflectcache.go +++ b/vendor/sigs.k8s.io/structured-merge-diff/v4/value/reflectcache.go @@ -269,7 +269,6 @@ func (e TypeReflectCacheEntry) FromUnstructured(sv, dv reflect.Value) error { if err != nil { return fmt.Errorf("error encoding %s to json: %v", st.String(), err) } - // TODO: this is where we're failing if unmarshaler, ok := e.getJsonUnmarshaler(dv); ok { return unmarshaler.UnmarshalJSON(data) }