Skip to content

Commit

Permalink
Merge pull request #1453 from tnozicka/fix-namespaced-must-gather
Browse files Browse the repository at this point in the history 
Fix namespaced must-gather
  • Loading branch information
@scylla-operator-bot
scylla-operator-bot[bot] authored Oct 8, 2023
2 parents 0aa7f90 + 5b752cc commit 084f044
Show file tree
Hide file tree
Showing 4 changed files with 154 additions and 13 deletions.
2 changes: 1 addition & 1 deletion pkg/cmd/operator/gather.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
16 changes: 8 additions & 8 deletions pkg/cmd/operator/gatherbase.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ import (
)

type GatherBaseOptions struct {
gathererName string
configFlags *kgenericclioptions.ConfigFlags
GathererName string
ConfigFlags *kgenericclioptions.ConfigFlags

kubeClient kubernetes.Interface
dynamicClient dynamic.Interface
Expand All @@ -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
Expand All @@ -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.")
Expand Down Expand Up @@ -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)
}
Expand All @@ -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)
Expand All @@ -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)
}
Expand Down
16 changes: 15 additions & 1 deletion pkg/cmd/operator/mustgather.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -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,
})
}
Expand Down
133 changes: 130 additions & 3 deletions pkg/cmd/operator/mustgather_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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{
Expand All @@ -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",
Expand All @@ -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"),
},
{
Expand All @@ -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{
Expand Down Expand Up @@ -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{
Expand All @@ -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",
Expand Down

0 comments on commit 084f044

Please sign in to comment.