Skip to content

Conversation

@akrem-chabchoub
Copy link
Contributor

@akrem-chabchoub akrem-chabchoub commented Aug 22, 2025

Refactor: Use NodeProvider interface for restart command

Problem

The restart command was tightly coupled to the orchestration package, making it difficult to reuse and maintain. It had separate methods for cluster and namespace-based restarts with duplicated logic.

Solution

  • Refactored to use NodeProvider interface for unified node discovery
  • Simplified restart client with single Restart() method
  • Added node group filtering capabilities
  • Added direct pod restart support for namespace-based operations
  • Improved error handling and logging

Changes

  • Restart Client Refactoring: Replaced orchestration dependency with NodeProvider interface
  • Node Group Filtering: Added support for filtering nodes by groups in beekeeper deployments
  • Command Simplification: Unified cluster and namespace restart logic
  • Pod-Level Restart: Added direct pod restart using label selectors for namespace operations
  • Deployment Type Support: Added --deployment-type flag for beekeeper vs helm deployments

Usage Examples

# Restart with new image
beekeeper restart --cluster-name=my-cluster --image=bee:v2.0.0

# Namespace-based restart with pod label selector
beekeeper restart --namespace=my-namespace --label-selector="type:type1"

Closes #526

@akrem-chabchoub akrem-chabchoub changed the title Refactor/restart cmd refactor: enhance restart command to utilize NodeProvider Aug 22, 2025
@akrem-chabchoub akrem-chabchoub marked this pull request as ready for review August 22, 2025 16:11
@akrem-chabchoub akrem-chabchoub changed the base branch from master to refactor/node-provider-interface August 25, 2025 10:16

return nil
}
cluster, err := c.setupCluster(ctx, clusterName, false)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

c.setupCluster(ctx, clusterName, false) is already called in c.createNodeClient(ctx, true), no need to call it twice.

akrem-chabchoub and others added 5 commits September 8, 2025 12:14
* feat: fize-sizes param for smoke

* ci: file-sizes in configs

* ci: file-sizes in staging

* refactor: comments

* refactor: backward compatibility for ContentSize

* fix: comments

* fix(load): use content-size for load check (#522)

---------

Co-authored-by: Ljubiša Gačević <35105035+gacevicljubisa@users.noreply.github.com>
#487)

* chore: add new config file and split nodes setup for create cluster

* chore: update config to use latest docker image
@gacevicljubisa
Copy link
Member

If image is not specified, pod should only be restarted. If specified, then update the Image of the statefull set and then pod should be deleted if strategy is UpdateStrategyOnDelete

); err != nil {
return fmt.Errorf("restarting cluster %s: %w", clusterName, err)
// TODO: Add cluster restart (should be handled by the node client)
if err := restartClient.RestartPods(ctx); err != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this method is enough to restart all bee nodes, we should rename it to Restart(), and remove RestartCluster()

// filterClientsByNodeGroups filters bee clients by node groups
// Node names in beekeeper deployments follow the pattern: {nodeGroup}-{index}
// This function extracts the node group from the node name and filters accordingly
func (sc *Client) filterClientsByNodeGroups(clients map[string]*bee.Client) map[string]*bee.Client {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You do not need this logic. Much simplier and more precise solution would be to add NodeGroupName in bee.ClientOptions, and to expose new method NodeGroup() in bee package. Then you can compare on name. This should be done only in case of beekeeper deployment.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is psudo code how this filtering could work:

	filteredClients := make(map[string]*bee.Client)
	if len(sc.nodeGroups) > 0 && sc.deploymentType == DeploymentTypeBeekeeper {
		for _, ng := range sc.nodeGroups {
			for name, client := range sc.beeClients {
				if client.NodeGroup() == ng {
					filteredClients[name] = client
				}
			}
		}
	} else {
		filteredClients = sc.beeClients
	}

@akrem-chabchoub
Copy link
Contributor Author

#516 (comment) , Its already implemented here:

func (c *Client) updateOrDeleteNode(ctx context.Context, node, ns, image string) error {

@gacevicljubisa gacevicljubisa merged commit e6244a5 into refactor/node-provider-interface Oct 3, 2025
3 checks passed
@gacevicljubisa gacevicljubisa deleted the refactor/restart-cmd branch October 3, 2025 10:51
gacevicljubisa added a commit that referenced this pull request Oct 5, 2025
#512)

* refactor: introduce NodeProvider interface for unified node management

* fix: do not check for deployment type in node-funder and improve logs

* refactor: enhance restart command to utilize NodeProvider (#516)

* refactor: enhance restart command to utilize NodeProvider

* refactor: add deployment-type flag to restart command

* docs: update restart command docs in README

* refactor: simplify restart command by removing unused cluster name and namespace checks

* fix: adjust node name handling for Ingress and IngressRoute nodes

* fix: change Service Port type to interface{} for better handling of string and number values (temp)

* lint: remove unnecessary blank line

* fix: update node discovery

* feat: smoke file sizes (#505)

* feat: fize-sizes param for smoke

* ci: file-sizes in configs

* ci: file-sizes in staging

* refactor: comments

* refactor: backward compatibility for ContentSize

* fix: comments

* fix(load): use content-size for load check (#522)

---------

Co-authored-by: Ljubiša Gačević <35105035+gacevicljubisa@users.noreply.github.com>

* refactor: add new config file and split nodes setup for create cluster (#487)

* chore: add new config file and split nodes setup for create cluster

* chore: update config to use latest docker image

* fix: add image flag to restart cmd

* refactor: add node groups support for filtering nodes

* refactor: enhance Client structure to include NodeGroupName and update filtering logic for beekeeper deployments

* refactor(node): improve filterClientsByNodeGroups method

---------

Co-authored-by: Ljubisa Gacevic <ljubisa.rs@gmail.com>
Co-authored-by: nugaon <toth.viktor.levente@gmail.com>
Co-authored-by: Ljubiša Gačević <35105035+gacevicljubisa@users.noreply.github.com>

* fix: minor changes

* fix: use node-grups for filtering

---------

Co-authored-by: Akrem Chabchoub <121046693+akrem-chabchoub@users.noreply.github.com>
Co-authored-by: nugaon <toth.viktor.levente@gmail.com>
@bcsorvasi bcsorvasi added this to the bee-v2.7.0 milestone Oct 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor: Enhance Node Management with NodeProvider Interface and Improve Restart Command

5 participants