From 5b752cca9f222188614191c4da1418955e915199 Mon Sep 17 00:00:00 2001 From: Tomas Nozicka Date: Fri, 6 Oct 2023 14:43:20 +0200 Subject: [PATCH] Fix namespaced must-gather --- pkg/cmd/operator/gather.go | 2 +- pkg/cmd/operator/gatherbase.go | 16 ++-- pkg/cmd/operator/mustgather.go | 16 +++- pkg/cmd/operator/mustgather_test.go | 133 +++++++++++++++++++++++++++- 4 files changed, 154 insertions(+), 13 deletions(-) diff --git a/pkg/cmd/operator/gather.go b/pkg/cmd/operator/gather.go index 8b68579f2d1..6e23a59b6ce 100644 --- a/pkg/cmd/operator/gather.go +++ b/pkg/cmd/operator/gather.go @@ -124,7 +124,7 @@ func (o *GatherOptions) Complete(args []string) error { return fmt.Errorf("can't complete gather base: %w", err) } - o.builder = o.builderFlags.ToBuilder(o.configFlags, args) + o.builder = o.builderFlags.ToBuilder(o.ConfigFlags, args) return nil } diff --git a/pkg/cmd/operator/gatherbase.go b/pkg/cmd/operator/gatherbase.go index 0bd816776f6..43881492e72 100644 --- a/pkg/cmd/operator/gatherbase.go +++ b/pkg/cmd/operator/gatherbase.go @@ -25,8 +25,8 @@ import ( ) type GatherBaseOptions struct { - gathererName string - configFlags *kgenericclioptions.ConfigFlags + GathererName string + ConfigFlags *kgenericclioptions.ConfigFlags kubeClient kubernetes.Interface dynamicClient dynamic.Interface @@ -40,8 +40,8 @@ type GatherBaseOptions struct { func NewGatherBaseOptions(gathererName string, keepGoing bool) *GatherBaseOptions { return &GatherBaseOptions{ - gathererName: gathererName, - configFlags: kgenericclioptions.NewConfigFlags(true).WithWrapConfigFn(func(c *rest.Config) *rest.Config { + GathererName: gathererName, + ConfigFlags: kgenericclioptions.NewConfigFlags(true).WithWrapConfigFn(func(c *rest.Config) *rest.Config { c.UserAgent = genericclioptions.MakeVersionedUserAgent(fmt.Sprintf("scylla-operator-%s", gathererName)) // Don't slow down artificially. c.QPS = math.MaxFloat32 @@ -56,7 +56,7 @@ func NewGatherBaseOptions(gathererName string, keepGoing bool) *GatherBaseOption } func (o *GatherBaseOptions) AddFlags(flagset *pflag.FlagSet) { - o.configFlags.AddFlags(flagset) + o.ConfigFlags.AddFlags(flagset) flagset.StringVarP(&o.DestDir, "dest-dir", "", o.DestDir, "Destination directory where to store the artifacts.") flagset.Int64VarP(&o.LogsLimitBytes, "log-limit-bytes", "", o.LogsLimitBytes, "Maximum number of bytes collected for each log file, 0 means unlimited.") @@ -90,7 +90,7 @@ func (o *GatherBaseOptions) Validate() error { } func (o *GatherBaseOptions) Complete() error { - restConfig, err := o.configFlags.ToRESTConfig() + restConfig, err := o.ConfigFlags.ToRESTConfig() if err != nil { return fmt.Errorf("can't create RESTConfig: %w", err) } @@ -108,7 +108,7 @@ func (o *GatherBaseOptions) Complete() error { o.discoveryClient = cacheddiscovery.NewMemCacheClient(o.kubeClient.Discovery()) if len(o.DestDir) == 0 { - o.DestDir = fmt.Sprintf("%s-%s", o.gathererName, utilrand.String(12)) + o.DestDir = fmt.Sprintf("%s-%s", o.GathererName, utilrand.String(12)) err := os.Mkdir(o.DestDir, 0770) if err != nil { return fmt.Errorf("can't create destination directory %q: %w", o.DestDir, err) @@ -130,7 +130,7 @@ func (o *GatherBaseOptions) RunInit(originalStreams genericclioptions.IOStreams, return fmt.Errorf("can't set alsologtostderr flag: %w", err) } - err = flag.Set("log_file", filepath.Join(o.DestDir, fmt.Sprintf("%s.log", o.gathererName))) + err = flag.Set("log_file", filepath.Join(o.DestDir, fmt.Sprintf("%s.log", o.GathererName))) if err != nil { return fmt.Errorf("can't set log_file flag: %w", err) } diff --git a/pkg/cmd/operator/mustgather.go b/pkg/cmd/operator/mustgather.go index 1fb0cfcb6ad..09674f5de99 100644 --- a/pkg/cmd/operator/mustgather.go +++ b/pkg/cmd/operator/mustgather.go @@ -12,6 +12,7 @@ import ( "github.com/spf13/pflag" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" utilerrors "k8s.io/apimachinery/pkg/util/errors" @@ -30,6 +31,12 @@ var ( mustGatherExample = templates.Examples(` # Collect archive of all resources related to scyllacluster and its APIs. scylla-operator must-gather + + # Collect archive of all resources related to scyllacluster and its APIs. + # Namespaced APIs targeted for collection will be limited to the supplied namespace. + # (E.g. this will limit ScyllaClusters to the supplied namespace, + # unless some other resource requests them collected.) + scylla-operator must-gather -n my-namespace # Collect archive of all resources present in the Kubernetes cluster. scylla-operator must-gather --all-resources @@ -262,9 +269,16 @@ func (o *MustGatherOptions) run(ctx context.Context) error { return fmt.Errorf("can't find resource in preferred resources: %w", err) } + namespace := s.Namespace + if ri.Scope.Name() == meta.RESTScopeNameNamespace && + s.Namespace == corev1.NamespaceAll && + o.GatherBaseOptions.ConfigFlags.Namespace != nil { + namespace = *o.GatherBaseOptions.ConfigFlags.Namespace + } + resourceSpecs = append(resourceSpecs, resourceSpec{ ResourceInfo: *ri, - Namespace: s.Namespace, + Namespace: namespace, Name: s.Name, }) } diff --git a/pkg/cmd/operator/mustgather_test.go b/pkg/cmd/operator/mustgather_test.go index 919dc4d0303..6a7fb8e88dc 100644 --- a/pkg/cmd/operator/mustgather_test.go +++ b/pkg/cmd/operator/mustgather_test.go @@ -22,6 +22,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" + kgenericclioptions "k8s.io/cli-runtime/pkg/genericclioptions" fakediscovery "k8s.io/client-go/discovery/fake" dynamicfakeclient "k8s.io/client-go/dynamic/fake" kubefakeclient "k8s.io/client-go/kubernetes/fake" @@ -89,11 +90,13 @@ func TestMustGatherOptions_Run(t *testing.T) { name string existingObjects []runtime.Object allResources bool + namespace string expectedDump *testhelpers.GatherDump expectedError error }{ { - name: "smoke test", + name: "gathers objects in all namespaces", + namespace: corev1.NamespaceAll, existingObjects: []runtime.Object{ &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ @@ -109,6 +112,18 @@ func TestMustGatherOptions_Run(t *testing.T) { "secret-key": []byte("secret-value"), }, }, + &scyllav1.ScyllaCluster{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "my-namespace", + Name: "scylla", + }, + }, + &scyllav1.ScyllaCluster{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "my-other-namespace", + Name: "other-scylla", + }, + }, &apiserverinternalv1alpha1.StorageVersion{ ObjectMeta: metav1.ObjectMeta{ Name: "my-non-standard-resource-that-should-not-be-collected", @@ -130,6 +145,44 @@ metadata: name: scylla-operator spec: {} status: {} +`, "\n"), + }, + { + Name: "namespaces/my-namespace/scyllaclusters.scylla.scylladb.com/scylla.yaml", + Content: strings.TrimPrefix(` +apiVersion: scylla.scylladb.com/v1 +kind: ScyllaCluster +metadata: + creationTimestamp: null + name: scylla + namespace: my-namespace +spec: + agentVersion: "" + datacenter: + name: "" + racks: null + network: {} + version: "" +status: {} +`, "\n"), + }, + { + Name: "namespaces/my-other-namespace/scyllaclusters.scylla.scylladb.com/other-scylla.yaml", + Content: strings.TrimPrefix(` +apiVersion: scylla.scylladb.com/v1 +kind: ScyllaCluster +metadata: + creationTimestamp: null + name: other-scylla + namespace: my-other-namespace +spec: + agentVersion: "" + datacenter: + name: "" + racks: null + network: {} + version: "" +status: {} `, "\n"), }, { @@ -152,7 +205,77 @@ metadata: }, }, { - name: "gathers all resources", + name: "gathers objects only in selected namespace", + namespace: "my-namespace", + existingObjects: []runtime.Object{ + &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "scylla-operator", + }, + }, + &scyllav1.ScyllaCluster{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "my-namespace", + Name: "scylla", + }, + }, + &scyllav1.ScyllaCluster{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "my-other-namespace", + Name: "do-not-collect", + }, + }, + &apiserverinternalv1alpha1.StorageVersion{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-non-standard-resource-that-should-not-be-collected", + }, + }, + }, + allResources: false, + expectedError: nil, + expectedDump: &testhelpers.GatherDump{ + EmptyDirs: nil, + Files: []testhelpers.File{ + { + Name: "cluster-scoped/namespaces/scylla-operator.yaml", + Content: strings.TrimPrefix(` +apiVersion: v1 +kind: Namespace +metadata: + creationTimestamp: null + name: scylla-operator +spec: {} +status: {} +`, "\n"), + }, + { + Name: "namespaces/my-namespace/scyllaclusters.scylla.scylladb.com/scylla.yaml", + Content: strings.TrimPrefix(` +apiVersion: scylla.scylladb.com/v1 +kind: ScyllaCluster +metadata: + creationTimestamp: null + name: scylla + namespace: my-namespace +spec: + agentVersion: "" + datacenter: + name: "" + racks: null + network: {} + version: "" +status: {} +`, "\n"), + }, + { + Name: "scylla-operator-must-gather.log", + }, + }, + }, + }, + { + name: "gathers all resources", + namespace: corev1.NamespaceAll, existingObjects: []runtime.Object{ &apiserverinternalv1alpha1.StorageVersion{ ObjectMeta: metav1.ObjectMeta{ @@ -214,7 +337,9 @@ status: {} tmpDir := t.TempDir() - fakeKubeClient := kubefakeclient.NewSimpleClientset(tc.existingObjects...) + // We don't actually list objects using this client and it can only work with native APIs, + // so we can leave it empty. + fakeKubeClient := kubefakeclient.NewSimpleClientset() fakeKubeClient.Resources = apiResources simpleFakeDiscoveryClient := fakeKubeClient.Discovery() fakeDiscoveryClient := &testhelpers.FakeDiscoveryWithSPR{ @@ -241,6 +366,8 @@ status: {} o.discoveryClient = fakeDiscoveryClient o.dynamicClient = fakeDynamicClient o.AllResources = tc.allResources + o.GatherBaseOptions.ConfigFlags = kgenericclioptions.NewConfigFlags(false) + *o.GatherBaseOptions.ConfigFlags.Namespace = tc.namespace cmd := &cobra.Command{ Use: "must-gather",