Repository: openshift/assisted-service (language: python)

Issue:
MGMT-17353: Debug pod left in ImagePullBackOff after install in disconnected environment

For day2 hosts the functionality is removed since we don't want to enable it for kube-api

Revert "[MGMT-15902](https://issues.redhat.com//browse/MGMT-15902): Trigger reboots for node event when day2 node moves to done (#5648)"

This reverts commit 652202cc520739d4e647c3a535ecdc4a806931c1.

<!--
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

You can refer to [Kubernetes community documentation] on writing good commit messages, which provides good tips and ideas.

Some PRs address specific issues. Please, refer to the [CONTRIBUTING] documentation for more
information on how to link a PR to an existing issue.

It's recommended to take a few extra minutes to provide more information about
how this code was tested. Here are some questions that may be worth answering:

- Should this PR be tested by the reviewer?
- Is this PR relying on CI for an e2e test run?
- Should this PR be tested in a specific environment?
- Any logs, screenshots, etc that can help with the review process?

-->

## List all the issues related to this PR

- [ ] New Feature <!-- new functionality -->
- [ ] Enhancement <!-- refactor, code changes, improvement, that won't add new features -->
- [x] Bug fix
- [ ] Tests
- [ ] Documentation
- [ ] CI/CD <!-- Notice that changes for Dockerfiles/Jenkinsfiles aren't tested in CI due to a known bug. -->

## What environments does this code impact?

- [ ] Automation (CI, tools, etc)
- [ ] Cloud
- [x] Operator Managed Deployments
- [ ] None

## How was this code tested?

<!-- Please, select one or more if needed: -->

- [ ] assisted-test-infra environment
- [ ] dev-scripts environment
- [ ] Reviewer's test appreciated
- [ ] Waiting for CI to do a full test run
- [ ] Manual (Elaborate on how it was tested)
- [x] No tests needed

## Checklist

- [ ] Title and description added to both, commit and PR.
- [ ] Relevant issues have been associated (see [CONTRIBUTING] guide)
- [ ] This change does not require a documentation update (docstring, `docs`, README, etc)
- [ ] Does this change include unit-tests (note that code changes require unit-tests)

## Reviewers Checklist

- Are the title and description (in both PR and commit) meaningful and clear?
- Is there a bug required (and linked) for this change?
- Should this PR be backported?

[Kubernetes community documentation]: https://github.com/kubernetes/community/blob/master/contributors/guide/pull-requests.md#commit-message-guidelines
[CONTRIBUTING]: https://github.com/openshift/assisted-service/blob/master/CONTRIBUTING.md
/cc @tsorya 


Discussion / hints:

Fix patch:
diff --git a/cmd/main.go b/cmd/main.go
index 016c31112e2..54206728c00 100644
--- a/cmd/main.go
+++ b/cmd/main.go
@@ -603,7 +603,6 @@ func main() {
 				ApproveCsrsRequeueDuration: Options.ApproveCsrsRequeueDuration,
 				AgentContainerImage:        Options.BMConfig.AgentDockerImg,
 				HostFSMountDir:             hostFSMountDir,
-				EventSender:                eventsHandler,
 			}).SetupWithManager(ctrlMgr), "unable to create controller Agent")
 
 			failOnError((&controllers.BMACReconciler{
diff --git a/go.mod b/go.mod
index 322ad171274..024420931d8 100644
--- a/go.mod
+++ b/go.mod
@@ -17,7 +17,6 @@ require (
 	github.com/danielerez/go-dns-client v0.0.0-20200630114514-0b60d1703f0b
 	github.com/dustin/go-humanize v1.0.1
 	github.com/filanov/stateswitch v1.0.1-0.20221122134945-bfa198e3a83a
-	github.com/go-errors/errors v1.4.2
 	github.com/go-gormigrate/gormigrate/v2 v2.0.1
 	github.com/go-logr/logr v1.2.4
 	github.com/go-openapi/errors v0.20.3
@@ -100,6 +99,7 @@ require (
 	github.com/emicklei/go-restful/v3 v3.11.0 // indirect
 	github.com/evanphx/json-patch/v5 v5.6.0 // indirect
 	github.com/exponent-io/jsonpath v0.0.0-20151013193312-d6023ce2651d // indirect
+	github.com/go-errors/errors v1.4.2 // indirect
 	github.com/go-logr/stdr v1.2.2 // indirect
 	github.com/go-sql-driver/mysql v1.7.0 // indirect
 	github.com/google/btree v1.0.1 // indirect
@@ -230,7 +230,7 @@ require (
 	go.uber.org/atomic v1.11.0 // indirect
 	go.uber.org/multierr v1.11.0 // indirect
 	go.uber.org/zap v1.24.0 // indirect
-	golang.org/x/net v0.24.0
+	golang.org/x/net v0.24.0 // indirect
 	golang.org/x/oauth2 v0.12.0 // indirect
 	golang.org/x/term v0.19.0 // indirect
 	golang.org/x/text v0.14.0 // indirect
diff --git a/internal/controller/controllers/agent_controller.go b/internal/controller/controllers/agent_controller.go
index 71a7a97bf06..18a1871897b 100644
--- a/internal/controller/controllers/agent_controller.go
+++ b/internal/controller/controllers/agent_controller.go
@@ -35,11 +35,8 @@ import (
 	aiv1beta1 "github.com/openshift/assisted-service/api/v1beta1"
 	"github.com/openshift/assisted-service/internal/bminventory"
 	"github.com/openshift/assisted-service/internal/common"
-	"github.com/openshift/assisted-service/internal/common/events"
-	eventsapi "github.com/openshift/assisted-service/internal/events/api"
 	"github.com/openshift/assisted-service/internal/gencrypto"
 	"github.com/openshift/assisted-service/internal/host"
-	"github.com/openshift/assisted-service/internal/oc"
 	"github.com/openshift/assisted-service/internal/spoke_k8s_client"
 	"github.com/openshift/assisted-service/models"
 	"github.com/openshift/assisted-service/pkg/auth"
@@ -93,7 +90,6 @@ type AgentReconciler struct {
 	AgentContainerImage        string
 	HostFSMountDir             string
 	reclaimer                  *agentReclaimer
-	EventSender                eventsapi.Sender
 }
 
 // +kubebuilder:rbac:groups=agent-install.openshift.io,resources=agents,verbs=get;list;watch;create;update;patch;delete
@@ -489,31 +485,6 @@ func (r *AgentReconciler) spokeKubeClient(ctx context.Context, clusterRef *aiv1b
 	return r.SpokeK8sClientFactory.CreateFromSecret(secret)
 }
 
-func (r *AgentReconciler) notifyNumberOfReboots(ctx context.Context, h *models.Host, node *corev1.Node, clusterRef *aiv1beta1.ClusterReference) error {
-	if !isNodeReady(node) {
-		return nil
-	}
-	secret, err := spokeKubeconfigSecret(ctx, r.Log, r.Client, r.APIReader, clusterRef)
-	if err != nil {
-		r.Log.WithError(err).Errorf("failed to get spoke secret for cluster %s/%s", clusterRef.Namespace, clusterRef.Name)
-		return err
-	}
-	kubeconfig, err := spoke_k8s_client.KubeconfigFromSecret(secret)
-	if err != nil {
-		r.Log.WithError(err).Errorf("failed to get kubeconfig from secret for cluster %s/%s", clusterRef.Namespace, clusterRef.Name)
-		return err
-	}
-	dbg := oc.NewDebug(kubeconfig)
-	reboots, err := dbg.RebootsForNode(node.Name)
-	if err != nil {
-		r.Log.WithError(err).Errorf("failed to get number of reboots for node %s, cluster %s/%s", node.Name,
-			clusterRef.Namespace, clusterRef.Name)
-		return err
-	}
-	events.SendRebootsForNodeEvent(ctx, r.EventSender, *h.ID, node.Name, h.InfraEnvID, h.ClusterID, int64(reboots))
-	return nil
-}
-
 // Attempt to approve CSRs for agent. If already approved then the node will be marked as done
 // requeue means that approval will be attempted again
 func (r *AgentReconciler) tryApproveDay2CSRs(ctx context.Context, agent *aiv1beta1.Agent, node *corev1.Node, client spoke_k8s_client.SpokeK8sClient) {
@@ -817,9 +788,6 @@ func (r *AgentReconciler) updateStatus(ctx context.Context, log logrus.FieldLogg
 					log.WithError(err).Errorf("Failed to apply labels for day2 node %s/%s", agent.Namespace, agent.Name)
 					return ctrl.Result{RequeueAfter: defaultRequeueAfterOnError}, err
 				}
-				if err = r.notifyNumberOfReboots(ctx, h, node, agent.Spec.ClusterDeploymentName); err != nil {
-					log.WithError(err).Errorf("Failed to notify number of reboots for day2 node %s/%s", agent.Namespace, agent.Name)
-				}
 				if err = r.UpdateDay2InstallPogress(ctx, h, agent, node); err != nil {
 					return ctrl.Result{RequeueAfter: defaultRequeueAfterOnError}, err
 				}
diff --git a/internal/oc/debug.go b/internal/oc/debug.go
deleted file mode 100644
index 3066447ba2f..00000000000
--- a/internal/oc/debug.go
+++ /dev/null
@@ -1,96 +0,0 @@
-package oc
-
-import (
-	"io"
-	"os"
-	"strings"
-	"time"
-
-	"github.com/go-errors/errors"
-	"github.com/openshift/assisted-service/pkg/executer"
-	"golang.org/x/net/context"
-)
-
-type Debug interface {
-	RebootsForNode(nodeName string) (int, error)
-}
-
-type debug struct {
-	kubeconfig []byte
-	exec       executer.Executer
-}
-
-func NewDebug(kubeconfig []byte) Debug {
-	return &debug{
-		kubeconfig: kubeconfig,
-		exec:       &executer.CommonExecuter{},
-	}
-}
-
-func NewDebugWithExecuter(kubeconfig []byte, exec executer.Executer) Debug {
-	return &debug{
-		kubeconfig: kubeconfig,
-		exec:       exec,
-	}
-}
-
-func (d *debug) kubeconfigFile() (string, error) {
-	var n int
-
-	f, err := os.CreateTemp("", "kubeconfig")
-	if err != nil {
-		return "", err
-	}
-	defer f.Close()
-	numWritten := 0
-	for n, err = f.Write(d.kubeconfig[numWritten:]); err == io.ErrShortWrite; n, err = f.Write(d.kubeconfig[numWritten:]) {
-		numWritten += n
-	}
-	if err != nil {
-		return "", err
-	}
-	return f.Name(), nil
-}
-
-func (d *debug) runDebug(entity string, args ...string) (string, string, error) {
-	kubeconfigFname, err := d.kubeconfigFile()
-	if err != nil {
-		return "", "", err
-	}
-	defer func() {
-		_ = os.RemoveAll(kubeconfigFname)
-	}()
-	execArgs := append([]string{
-		"debug",
-		"--kubeconfig",
-		kubeconfigFname,
-		entity,
-		"--",
-	}, args...)
-	execCtx, cancel := context.WithTimeout(context.TODO(), 2*time.Minute)
-	defer cancel()
-	out, errOut, exitCode := d.exec.ExecuteWithContext(execCtx, "oc", execArgs...)
-	if exitCode != 0 {
-		return "", "", errors.Errorf("oc debug failed to execute with code %d: %s", exitCode, errOut)
-	}
-	return out, errOut, nil
-}
-
-func (d *debug) RebootsForNode(nodeName string) (int, error) {
-	out, _, err := d.runDebug("node/"+nodeName,
-		"chroot",
-		"/host",
-		"last",
-		"reboot")
-	if err != nil {
-		return 0, err
-	}
-	lines := strings.Split(out, "\n")
-	numReboots := 0
-	for _, line := range lines {
-		if strings.HasPrefix(line, "reboot ") {
-			numReboots++
-		}
-	}
-	return numReboots, nil
-}
diff --git a/internal/spoke_k8s_client/factory.go b/internal/spoke_k8s_client/factory.go
index 8939864189b..9c6d403a101 100644
--- a/internal/spoke_k8s_client/factory.go
+++ b/internal/spoke_k8s_client/factory.go
@@ -41,7 +41,7 @@ func (cf *spokeK8sClientFactory) CreateFromRawKubeconfig(kubeconfig []byte) (Spo
 }
 
 func (cf *spokeK8sClientFactory) CreateFromSecret(secret *corev1.Secret) (SpokeK8sClient, error) {
-	kubeconfigData, err := KubeconfigFromSecret(secret)
+	kubeconfigData, err := kubeconfigFromSecret(secret)
 	if err != nil {
 		return nil, err
 	}
@@ -66,7 +66,7 @@ func (cf *spokeK8sClientFactory) CreateFromStorageKubeconfig(ctx context.Context
 }
 
 func (cf *spokeK8sClientFactory) ClientAndSetFromSecret(secret *corev1.Secret) (SpokeK8sClient, *kubernetes.Clientset, error) {
-	kubeconfig, err := KubeconfigFromSecret(secret)
+	kubeconfig, err := kubeconfigFromSecret(secret)
 	if err != nil {
 		cf.log.WithError(err).Error("failed to get kubeconfig from secret")
 		return nil, nil, err
@@ -75,7 +75,7 @@ func (cf *spokeK8sClientFactory) ClientAndSetFromSecret(secret *corev1.Secret) (
 	return cf.clientAndSetForKubeconfig(kubeconfig)
 }
 
-func KubeconfigFromSecret(secret *corev1.Secret) ([]byte, error) {
+func kubeconfigFromSecret(secret *corev1.Secret) ([]byte, error) {
 	if secret.Data == nil {
 		return nil, errors.Errorf("Secret %s/%s  does not contain any data", secret.Namespace, secret.Name)
 	}


Test patch:
diff --git a/internal/oc/debug_test.go b/internal/oc/debug_test.go
deleted file mode 100644
index 85f6102d435..00000000000
--- a/internal/oc/debug_test.go
+++ /dev/null
@@ -1,68 +0,0 @@
-package oc
-
-import (
-	"fmt"
-
-	"github.com/golang/mock/gomock"
-	. "github.com/onsi/ginkgo"
-	. "github.com/onsi/gomega"
-	"github.com/openshift/assisted-service/pkg/executer"
-)
-
-var _ = Describe("oc debug", func() {
-	const (
-		kubeconfig = "kubeconfig"
-		nodeName   = "node1"
-	)
-	var (
-		ctrl     *gomock.Controller
-		execMock *executer.MockExecuter
-		d        Debug
-	)
-
-	BeforeEach(func() {
-		ctrl = gomock.NewController(GinkgoT())
-		execMock = executer.NewMockExecuter(ctrl)
-		d = NewDebugWithExecuter([]byte(kubeconfig), execMock)
-	})
-	expectOk := func(ret string) {
-		execMock.EXPECT().ExecuteWithContext(gomock.Any(),
-			"oc",
-			"debug",
-			"--kubeconfig",
-			gomock.Any(),
-			fmt.Sprintf("node/%s", nodeName),
-			"--",
-			"chroot",
-			"/host",
-			"last",
-			"reboot").Return(ret, "", 0)
-	}
-	It("1 reboot", func() {
-		expectOk("reboot   system boot  4.18.0-372.9.1.e Tue Mar  7 04:13   still running\n")
-		numReboots, err := d.RebootsForNode(nodeName)
-		Expect(err).ToNot(HaveOccurred())
-		Expect(numReboots).To(Equal(1))
-	})
-	It("2 reboot", func() {
-		expectOk("reboot   system boot  4.18.0-372.9.1.e Tue Mar  7 04:13   still running\nreboot   system boot  4.18.0-372.9.1.e Sun Mar  5 07:29 - 09:11 (2+01:41)\n")
-		numReboots, err := d.RebootsForNode(nodeName)
-		Expect(err).ToNot(HaveOccurred())
-		Expect(numReboots).To(Equal(2))
-	})
-	It("with error", func() {
-		execMock.EXPECT().ExecuteWithContext(gomock.Any(),
-			"oc",
-			"debug",
-			"--kubeconfig",
-			gomock.Any(),
-			fmt.Sprintf("node/%s", nodeName),
-			"--",
-			"chroot",
-			"/host",
-			"last",
-			"reboot").Return("", "This is an error", -1)
-		_, err := d.RebootsForNode(nodeName)
-		Expect(err).To(HaveOccurred())
-	})
-})