Merge pull request #666 from typeid/fixes_acm_mg

Maint: remove dependency on "oc" for the osdctl hcp must-gather command

cmd/hcp/mustgather/mustGather.go

@@ -4,44 +4,36 @@ import (
 	"archive/tar"
 	"bytes"
 	"compress/gzip"
+	"context"
 	"fmt"
 	"io"
 	"net/http"
 	"net/url"
 	"os"
+	"os/exec"
+	"os/signal"
 	"path/filepath"
 	"strings"
 	"sync"
+	"syscall"
 	"time"
 
-	configclient "github.com/openshift/client-go/config/clientset/versioned"
-	imagev1client "github.com/openshift/client-go/image/clientset/versioned/typed/image/v1"
-	"github.com/openshift/oc/pkg/cli/admin/mustgather"
 	"github.com/openshift/osdctl/cmd/common"
 	"github.com/openshift/osdctl/cmd/dynatrace"
 	"github.com/openshift/osdctl/pkg/osdctlConfig"
 	"github.com/openshift/osdctl/pkg/utils"
 	"github.com/spf13/cobra"
-	"k8s.io/cli-runtime/pkg/genericclioptions"
-	"k8s.io/cli-runtime/pkg/genericiooptions"
-	"k8s.io/cli-runtime/pkg/printers"
-	"k8s.io/client-go/dynamic"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/clientcmd"
 	clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
-	kcmdutil "k8s.io/kubectl/pkg/cmd/util"
-	"k8s.io/kubectl/pkg/scheme"
-)
-
-var (
-	acmMustGatherImage = "quay.io/stolostron/must-gather:2.11.4-SNAPSHOT-2024-12-02-15-19-44"
 )
 
 type mustGather struct {
-	clusterId     string
-	reason        string
-	gatherTargets string
+	clusterId          string
+	reason             string
+	gatherTargets      string
+	acmMustGatherImage string
 }
 
 func NewCmdMustGather() *cobra.Command {
@@ -60,8 +52,10 @@ func NewCmdMustGather() *cobra.Command {
 		},
 	}
 
+	defaultAcmImage := "quay.io/stolostron/must-gather:2.11.4-SNAPSHOT-2024-12-02-15-19-44"
 	mustGatherCommand.Flags().StringVar(&mg.reason, "reason", "", "The reason for this command, which requires elevation (e.g., OHSS ticket or PD incident).")
 	mustGatherCommand.Flags().StringVar(&mg.gatherTargets, "gather", "hcp", "Comma-separated list of gather targets (available: sc_mg, sc_acm, mc_mg, mc_acm, hc, hcp).")
+	mustGatherCommand.Flags().StringVar(&mg.acmMustGatherImage, "acm_image", defaultAcmImage, "Overrides the acm must-gather image being used for acm mc, sc as well as hcp must-gathers.")
 
 	mustGatherCommand.MarkFlagRequired("reason")
 
@@ -165,18 +159,21 @@ func (mg *mustGather) Run() error {
 					fmt.Printf("failed to gather %s: %v\n", gatherTarget, err)
 				}
 			case "sc_acm":
-				if err := createMustGather(scRestCfg, scK8sCli, outputDir+"/sc_acm", acmMustGatherImage); err != nil {
+				if err := createMustGather(scRestCfg, scK8sCli, outputDir+"/sc_acm", mg.acmMustGatherImage); err != nil {
 					fmt.Printf("failed to gather %s: %v\n", gatherTarget, err)
 				}
 			case "mc_mg":
 				if err := createMustGather(mcRestCfg, mcK8sCli, outputDir+"/mc_infra", ""); err != nil {
 					fmt.Printf("failed to gather %s: %v\n", gatherTarget, err)
 				}
 			case "mc_acm":
-				if err := createMustGather(mcRestCfg, mcK8sCli, outputDir+"/mc_acm", acmMustGatherImage); err != nil {
+				if err := createMustGather(mcRestCfg, mcK8sCli, outputDir+"/mc_acm", mg.acmMustGatherImage); err != nil {
 					fmt.Printf("failed to gather %s: %v\n", gatherTarget, err)
 				}
 			case "hcp":
+				// TODO(typeid): add ACM must-gather to dump the HCP yamls, following https://issues.redhat.com/browse/ACM-16170
+				fmt.Println("WARNING: must-gather target `hcp` currently only gathers logs. Please gather the hypershift dump manually using the hypershift binary. In the near future, this will be automated as the ACM image will contain the dump binary.")
+
 				gatherOptions := &dynatrace.GatherLogsOpts{Since: 72, SortOrder: "asc", DestDir: outputDir + "/hcp_logs_dump"}
 				if err := gatherOptions.GatherLogs(mg.clusterId); err != nil {
 					fmt.Printf("failed to gather HCP dynatrace logs: %v\n", err)
@@ -216,63 +213,39 @@ func (mg *mustGather) Run() error {
 }
 
 func createMustGather(restCfg *rest.Config, k8sCli *kubernetes.Clientset, destDir, image string) error {
-	// hack(typeid): there's two major issues with using the oc cli's must gather:
-	// 1) oc cli assumes the client used for rsh and will default to a "sibling" command.
-	// This results in oc's rsync (used to copy files from a must-gather pod) to used `oc rsh`
-	// See here: https://github.com/openshift/oc/blob/cbc8edcc9c58cd8b2238d01e55ae8c8b0979ec4e/pkg/cli/rsync/copy_rsync.go#L35
-	// In our case, the default rsync remote shell gets resolved to standard rsh, which does not work as we are passing
-	// parameters like --namespace!
-	// 2) oc cli shells out to call oc rsh (in our case, just rsh) and assumes the kubeconfig.
-	// As we are running must gathers for multiple clusters, we need to make sure we're using the right kubeconfigs
-	// that we created programmatically, vs the one we currently have as a file locally.
-	// See here: https://github.com/openshift/oc/blob/cbc8edcc9c58cd8b2238d01e55ae8c8b0979ec4e/pkg/cli/rsync/copy_rsync.go#L84
-	// WORKAROUND:
-	// [1] we are overriding the `o.RsyncRshCmd` to use `oc rsh` vs `rsh`.
-	// For this, we need to use `mustgather.NewGatherOptions()` vs `mustgather.NewGatherCommand`, which adds a lot of bloat code.
-	// [2] we are passing a kubeconfig parameter, for that, we create a temporary kubeconfig from retrieved restConfig.
-
-	// We're not printing those out, as the logs are in the must-gather output.
-	var stdout, stderr bytes.Buffer
-	streams := genericiooptions.IOStreams{
-		In:     nil,
-		Out:    &stdout,
-		ErrOut: &stderr,
-	}
-
+	// We used to run this programatically by directly using the must-gather package  (see https://github.com/openshift/osdctl/pull/660)
+	// from the oc cli, but decided to opt for oc.Exec instead.
+	// Reasoning:
+	// 1) the must-gather internals, even when called programmatically, already shelled out to `oc rsync`.
+	// We had to hack around this by overriding the shell out with a specific kubeconfig.
+	// 2) the vendored dependencies of oc were causing issues with `go list`, see https://github.com/openshift/osdctl/pull/665.
 	kubeConfigFile := createKubeconfigFileForRestConfig(restCfg)
 	defer os.Remove(kubeConfigFile)
 
-	o := mustgather.NewMustGatherOptions(streams)
-	o.Client = k8sCli
-	o.Images = []string{image}
-	o.DestDir = destDir
-	o.RsyncRshCmd = fmt.Sprintf("oc rsh --kubeconfig=%s", kubeConfigFile)
-	o.Config = restCfg
-	flags := genericclioptions.NewConfigFlags(false)
-	o.RESTClientGetter = kcmdutil.NewFactory(flags)
-
-	var err error
-	if o.ConfigClient, err = configclient.NewForConfig(restCfg); err != nil {
-		return err
-	}
-	if o.DynamicClient, err = dynamic.NewForConfig(restCfg); err != nil {
-		return err
-	}
-	if o.ImageClient, err = imagev1client.NewForConfig(restCfg); err != nil {
-		return err
-	}
+	// Handle sigints and sigterms
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
 
-	o.PrinterCreated, err = printers.NewTypeSetter(scheme.Scheme).WrapToPrinter(&printers.NamePrinter{Operation: "created"}, nil)
-	if err != nil {
-		return err
-	}
-	o.PrinterDeleted, err = printers.NewTypeSetter(scheme.Scheme).WrapToPrinter(&printers.NamePrinter{Operation: "deleted"}, nil)
-	if err != nil {
-		return err
-	}
+	signalChan := make(chan os.Signal, 1)
+	signal.Notify(signalChan, syscall.SIGINT, syscall.SIGTERM)
+	go func() {
+		<-signalChan
+		fmt.Println("Received interrupt signal, canceling operation...")
+		cancel()
+	}()
 
-	if err := o.Run(); err != nil {
-		return err
+	cmd := exec.CommandContext(ctx, "oc", "adm", "must-gather", "--image="+image, "--dest-dir="+destDir, "--kubeconfig="+kubeConfigFile)
+
+	var stdout, stderr bytes.Buffer
+	cmd.Stdout = &stdout
+	cmd.Stderr = &stderr
+
+	err := cmd.Run()
+	if err != nil {
+		if ctx.Err() == context.Canceled {
+			return fmt.Errorf("command was canceled by user (e.g., Ctrl+C): %v\nstderr: %s", err, stderr.String())
+		}
+		return fmt.Errorf("failed to run 'oc adm must-gather': %v\nstderr: %s", err, stderr.String())
 	}
 
 	return nil

go.mod

@@ -39,12 +39,10 @@ require (
 	github.com/openshift/api v0.0.0-20250207102212-9e59a77ed2e0
 	github.com/openshift/aws-account-operator/api v0.0.0-20250205151445-6455c35fc4ae
 	github.com/openshift/backplane-cli v0.1.39
-	github.com/openshift/client-go v0.0.0-20250131180035-f7ec47e2d87a
 	github.com/openshift/cloud-credential-operator v0.0.0-20250120201329-db5f2531a5b4
 	github.com/openshift/gcp-project-operator v0.0.0-20241024143818-ec4eabd35aba
 	github.com/openshift/hive/apis v0.0.0-20250206153200-5a34ea42e678
 	github.com/openshift/hypershift/api v0.0.0-20250208145556-2753dcc8cfb7
-	github.com/openshift/oc v0.0.0-alpha.0.0.20250128192354-6f459acbc831
 	github.com/openshift/osd-network-verifier v1.2.2
 	github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c
 	github.com/shopspring/decimal v1.4.0
@@ -117,13 +115,11 @@ require (
 	github.com/danieljoos/wincred v1.2.2 // indirect
 	github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
 	github.com/deepmap/oapi-codegen v1.16.3 // indirect
-	github.com/docker/go-units v0.5.0 // indirect
 	github.com/dvsekhvalnov/jose2go v1.8.0 // indirect
 	github.com/emicklei/go-restful/v3 v3.12.1 // indirect
 	github.com/evanphx/json-patch v5.9.11+incompatible // indirect
 	github.com/evanphx/json-patch/v5 v5.9.11 // indirect
 	github.com/exponent-io/jsonpath v0.0.0-20210407135951-1de76d718b3f // indirect
-	github.com/fatih/camelcase v1.0.0 // indirect
 	github.com/fatih/structs v1.1.0 // indirect
 	github.com/felixge/httpsnoop v1.0.4 // indirect
 	github.com/flosch/pongo2/v4 v4.0.2 // indirect
@@ -214,7 +210,6 @@ require (
 	github.com/oasdiff/yaml3 v0.0.0-20241214160948-977117996672 // indirect
 	github.com/openshift/backplane-api v0.0.0-20241127094828-0f66644ff53d // indirect
 	github.com/openshift/custom-resource-status v1.1.3-0.20220503160415-f2fdb4999d87 // indirect
-	github.com/openshift/library-go v0.0.0-20250203131244-80620876b7c2 // indirect
 	github.com/pelletier/go-toml/v2 v2.2.3 // indirect
 	github.com/perimeterx/marshmallow v1.1.5 // indirect
 	github.com/peterbourgon/diskv v2.0.1+incompatible // indirect
@@ -277,7 +272,6 @@ require (
 	k8s.io/component-base v0.32.1 // indirect
 	k8s.io/klog/v2 v2.130.1 // indirect
 	k8s.io/kube-openapi v0.0.0-20241212222426-2c72e554b1e7 // indirect
-	k8s.io/pod-security-admission v0.32.1 // indirect
 	sigs.k8s.io/json v0.0.0-20241014173422-cfa47c3a1cc8 // indirect
 	sigs.k8s.io/kustomize/api v0.19.0 // indirect
 	sigs.k8s.io/structured-merge-diff/v4 v4.5.0 // indirect

go.sum

@@ -160,8 +160,6 @@ github.com/deckarep/golang-set v1.8.0 h1:sk9/l/KqpunDwP7pSjUg0keiOOLEnOBHzykLrsP
 github.com/deckarep/golang-set v1.8.0/go.mod h1:5nI87KwE7wgsBU1F4GKAw2Qod7p5kyS383rP6+o6qqo=
 github.com/deepmap/oapi-codegen v1.16.3 h1:GT9G86SbQtT1r8ZB+4Cybi9VGdu1P5ieNvNdEoCSbrA=
 github.com/deepmap/oapi-codegen v1.16.3/go.mod h1:JD6ErqeX0nYnhdciLc61Konj3NBASREMlkHOgHn8WAM=
-github.com/docker/go-units v0.5.0 h1:69rxXcBk27SvSaaxTtLh/8llcHD8vYHT7WSdRZ/jvr4=
-github.com/docker/go-units v0.5.0/go.mod h1:fgPhTUdO+D/Jk86RDLlptpiXQzgHJF7gydDDbaIK4Dk=
 github.com/docopt/docopt-go v0.0.0-20180111231733-ee0de3bc6815/go.mod h1:WwZ+bS3ebgob9U8Nd0kOddGdZWjyMGR8Wziv+TBNwSE=
 github.com/dvsekhvalnov/jose2go v1.8.0 h1:LqkkVKAlHFfH9LOEl5fe4p/zL02OhWE7pCufMBG2jLA=
 github.com/dvsekhvalnov/jose2go v1.8.0/go.mod h1:QsHjhyTlD/lAVqn/NSbVZmSCGeDehTB/mPZadG+mhXU=
@@ -180,8 +178,6 @@ github.com/evanphx/json-patch/v5 v5.9.11 h1:/8HVnzMq13/3x9TPvjG08wUGqBTmZBsCWzjT
 github.com/evanphx/json-patch/v5 v5.9.11/go.mod h1:3j+LviiESTElxA4p3EMKAB9HXj3/XEtnUf6OZxqIQTM=
 github.com/exponent-io/jsonpath v0.0.0-20210407135951-1de76d718b3f h1:Wl78ApPPB2Wvf/TIe2xdyJxTlb6obmF18d8QdkxNDu4=
 github.com/exponent-io/jsonpath v0.0.0-20210407135951-1de76d718b3f/go.mod h1:OSYXu++VVOHnXeitef/D8n/6y4QV8uLHSFXX4NeXMGc=
-github.com/fatih/camelcase v1.0.0 h1:hxNvNX/xYBp0ovncs8WyWZrOrpBNub/JfaMvbURyft8=
-github.com/fatih/camelcase v1.0.0/go.mod h1:yN2Sb0lFhZJUdVvtELVWefmrXpuZESvPmqwoZc+/fpc=
 github.com/fatih/color v1.9.0/go.mod h1:eQcE1qtQxscV5RaZvpXrrb8Drkc3/DdQ+uUYCNjL+zU=
 github.com/fatih/color v1.18.0 h1:S8gINlzdQ840/4pfAwic/ZE0djQEH3wM94VfqLTZcOM=
 github.com/fatih/color v1.18.0/go.mod h1:4FelSpRwEGDpQ12mAdzqdOukCy4u8WUtOY6lkT/6HfU=
@@ -428,8 +424,6 @@ github.com/leodido/go-urn v1.4.0 h1:WT9HwE9SGECu3lg4d/dIA+jxlljEa1/ffXKmRjqdmIQ=
 github.com/leodido/go-urn v1.4.0/go.mod h1:bvxc+MVxLKB4z00jd1z+Dvzr47oO32F/QSNjSBOlFxI=
 github.com/liggitt/tabwriter v0.0.0-20181228230101-89fcab3d43de h1:9TO3cAIGXtEhnIaL+V+BEER86oLrvS+kWobKpbJuye0=
 github.com/liggitt/tabwriter v0.0.0-20181228230101-89fcab3d43de/go.mod h1:zAbeS9B/r2mtpb6U+EI2rYA5OAXxsYw6wTamcNW+zcE=
-github.com/lithammer/dedent v1.1.0 h1:VNzHMVCBNG1j0fh3OrsFRkVUwStdDArbgBWoPAffktY=
-github.com/lithammer/dedent v1.1.0/go.mod h1:jrXYCQtgg0nJiN+StA2KgR7w6CiQNv9Fd/Z9BP0jIOc=
 github.com/magiconair/properties v1.8.9 h1:nWcCbLq1N2v/cpNsy5WvQ37Fb+YElfq20WJ/a8RkpQM=
 github.com/magiconair/properties v1.8.9/go.mod h1:Dhd985XPs7jluiymwWYZ0G4Z61jb3vdS329zhj2hYo0=
 github.com/mailgun/raymond/v2 v2.0.48 h1:5dmlB680ZkFG2RN/0lvTAghrSxIESeu9/2aeDqACtjw=
@@ -527,8 +521,6 @@ github.com/openshift/backplane-api v0.0.0-20241127094828-0f66644ff53d h1:7Dx+1tK
 github.com/openshift/backplane-api v0.0.0-20241127094828-0f66644ff53d/go.mod h1:RuJZpJy45AJnkp7A0ZPTZhLOVkCGDLI6cGknKvp65LE=
 github.com/openshift/backplane-cli v0.1.39 h1:KcqXeUJv6QSi/wTXP7X6leoR/fbsjh7JqCUIJ7qeJM4=
 github.com/openshift/backplane-cli v0.1.39/go.mod h1:lJHA3bn47vcEdpPCTXFgQbpukDCfzt/6eBO1c55Zr+k=
-github.com/openshift/client-go v0.0.0-20250131180035-f7ec47e2d87a h1:duO3JMrUOqVx50QhzxvDeOYIwTNOB8/EEuRLPyvAMBg=
-github.com/openshift/client-go v0.0.0-20250131180035-f7ec47e2d87a/go.mod h1:Qw3ThpzVZ0bfTILpBNYg4LGyjtNxfyCiGh/uDLOOTP8=
 github.com/openshift/cloud-credential-operator v0.0.0-20250120201329-db5f2531a5b4 h1:nrD3npDGt5bvwNXZKTzzEuZTI/4Uo5PbrkpAjfxhxtE=
 github.com/openshift/cloud-credential-operator v0.0.0-20250120201329-db5f2531a5b4/go.mod h1:Lzu29TMne5LsgPnyw2n9jrPiD5t6uyG5aE6KFy8cz6w=
 github.com/openshift/custom-resource-status v1.1.3-0.20220503160415-f2fdb4999d87 h1:cHyxR+Y8rAMT6m1jQCaYGRwikqahI0OjjUDhFNf3ySQ=
@@ -539,10 +531,6 @@ github.com/openshift/hive/apis v0.0.0-20250206153200-5a34ea42e678 h1:oPNt/o9il5M
 github.com/openshift/hive/apis v0.0.0-20250206153200-5a34ea42e678/go.mod h1:vfgOsNigipl8aM05Jy9WG+eR9wfeZQNEN4pTD9wsrtM=
 github.com/openshift/hypershift/api v0.0.0-20250208145556-2753dcc8cfb7 h1:535PgO4fBROL+cLcWJFH8aNyGPts9UgR42CV+N/pxGc=
 github.com/openshift/hypershift/api v0.0.0-20250208145556-2753dcc8cfb7/go.mod h1:fQFj8aH3buOKqmhMQ5igRVOT7iQdduxRE9H1LM/BiY0=
-github.com/openshift/library-go v0.0.0-20250203131244-80620876b7c2 h1:LaJmI+EeFnpbxs5H4Y6KOiNXhtPjldqvNzVfPpNM+dI=
-github.com/openshift/library-go v0.0.0-20250203131244-80620876b7c2/go.mod h1:GHwvopE5KXXCz4ULHp871sTPLLW+FB+hu/RIzlNwxx8=
-github.com/openshift/oc v0.0.0-alpha.0.0.20250128192354-6f459acbc831 h1:SStFXS3codvChRLRj7PbOt62NZDUbYGqzsfRQTyHTrs=
-github.com/openshift/oc v0.0.0-alpha.0.0.20250128192354-6f459acbc831/go.mod h1:RFjifQuYP0XDdZCFKqg4fL+7foKUQbfhuGL7ktxVKGs=
 github.com/openshift/osd-network-verifier v1.2.2 h1:A2RavUEM/bFGpxH848+pYAPMIIoVxJpx3Xm7BSmTvys=
 github.com/openshift/osd-network-verifier v1.2.2/go.mod h1:X3dVNkC91NYTf2kTXUS/PeRTNvfS97WbvIqPIDP083M=
 github.com/pelletier/go-toml/v2 v2.2.3 h1:YmeHyLY8mFWbdkNWwpr+qIL2bEqT0o95WSdkNHvL12M=
@@ -927,8 +915,6 @@ k8s.io/kube-openapi v0.0.0-20241212222426-2c72e554b1e7 h1:hcha5B1kVACrLujCKLbr8X
 k8s.io/kube-openapi v0.0.0-20241212222426-2c72e554b1e7/go.mod h1:GewRfANuJ70iYzvn+i4lezLDAFzvjxZYK1gn1lWcfas=
 k8s.io/kubectl v0.32.1 h1:/btLtXLQUU1rWx8AEvX9jrb9LaI6yeezt3sFALhB8M8=
 k8s.io/kubectl v0.32.1/go.mod h1:sezNuyWi1STk4ZNPVRIFfgjqMI6XMf+oCVLjZen/pFQ=
-k8s.io/pod-security-admission v0.32.1 h1:jcQjcxSwMsqcnr8ADiYe3Yhts0zEvY8BPEIFY6ducxU=
-k8s.io/pod-security-admission v0.32.1/go.mod h1:psSkvN+noAracLrouPjVDID/7TiMWoHQLNoBTVCY/nw=
 k8s.io/utils v0.0.0-20210802155522-efc7438f0176/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA=
 k8s.io/utils v0.0.0-20211116205334-6203023598ed/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA=
 k8s.io/utils v0.0.0-20241210054802-24370beab758 h1:sdbE21q2nlQtFh65saZY+rRM6x6aJJI8IUa1AmH/qa0=