Skip to content

Commit

Permalink
review + remove notes
Browse files Browse the repository at this point in the history
  • Loading branch information
nicmorales9 committed Apr 18, 2024
1 parent c440156 commit 73efaf7
Show file tree
Hide file tree
Showing 10 changed files with 9 additions and 14 deletions.
1 change: 0 additions & 1 deletion controllers/bounce_processes.go
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,6 @@ func checkIfClusterControllerNeedsRestart(logger logr.Logger, cluster *fdbv1beta
// We have to validate if at least one tester process is unreachable. In this case we have to restart the cluster
// controller. This will cause a recovery and the missing tester process will be removed from the list of unreachable
// processes.
// cross-DC since we kill using fdbcli regardless of DC
for _, process := range status.Cluster.Processes {
if process.ProcessClass == fdbv1beta2.ProcessClassTest {
if _, ok := unreachableProcessesSet[process.Address.String()]; ok {
Expand Down
1 change: 0 additions & 1 deletion controllers/change_coordinators.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,6 @@ func (c changeCoordinators) reconcile(ctx context.Context, r *FoundationDBCluste
// selectCandidates is a helper for Reconcile that picks non-excluded, not-being-removed class-matching process groups.
func selectCandidates(cluster *fdbv1beta2.FoundationDBCluster, status *fdbv1beta2.FoundationDBStatus) ([]locality.Info, error) {
candidates := make([]locality.Info, 0, len(status.Cluster.Processes))
// used cross-DC
for _, process := range status.Cluster.Processes {
if process.Excluded || process.UnderMaintenance {
continue
Expand Down
4 changes: 3 additions & 1 deletion controllers/choose_removals.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,10 @@ func (c chooseRemovals) reconcile(ctx context.Context, r *FoundationDBClusterRec
}

localityMap := make(map[string]locality.Info)
// should be cross-DC; this seems to choose what processes to keep based on cross-dc data
for _, process := range status.Cluster.Processes {
if cluster.Spec.DataCenter != process.Locality[fdbv1beta2.FDBLocalityDCIDKey] {
continue
}
id := process.Locality[fdbv1beta2.FDBLocalityInstanceIDKey]
localityMap[id] = locality.Info{ID: id, Address: process.Address, LocalityData: process.Locality}
}
Expand Down
8 changes: 5 additions & 3 deletions controllers/remove_incompatible_processes.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func processIncompatibleProcesses(ctx context.Context, r *FoundationDBClusterRec

logger.Info("incompatible connections", "incompatibleConnections", status.Cluster.IncompatibleConnections)

incompatibleConnections := parseIncompatibleConnections(logger, status)
incompatibleConnections := parseIncompatibleConnections(logger, status, cluster.Spec.DataCenter)
incompatiblePods := make([]*corev1.Pod, 0, len(incompatibleConnections))
for _, processGroup := range cluster.Status.ProcessGroups {
pod, err := r.PodLifecycleManager.GetPod(ctx, r, cluster, processGroup.GetPodName(cluster))
Expand Down Expand Up @@ -111,10 +111,12 @@ func processIncompatibleProcesses(ctx context.Context, r *FoundationDBClusterRec

// parseIncompatibleConnections parses the incompatible connections string slice to a map and removes all false reported incompatible processes.
// If a process is still part of the cluster status we can assume it's not an incompatible process.
func parseIncompatibleConnections(logger logr.Logger, status *fdbv1beta2.FoundationDBStatus) map[string]fdbv1beta2.None {
func parseIncompatibleConnections(logger logr.Logger, status *fdbv1beta2.FoundationDBStatus, dataCenter string) map[string]fdbv1beta2.None {
processAddressMap := map[string]fdbv1beta2.None{}
for _, process := range status.Cluster.Processes {
// TODO function sounds like it should work cross-DC, but the reconciler works on pods. Toss up to me since we'd need to thread DC in
if dataCenter != process.Locality[fdbv1beta2.FDBLocalityDCIDKey] {
continue
}
processAddressMap[process.Address.MachineAddress()] = fdbv1beta2.None{}
}

Expand Down
2 changes: 1 addition & 1 deletion controllers/remove_incompatible_processes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ var _ = Describe("restart_incompatible_pods", func() {
)

DescribeTable("when parsing incompatible connections", func(status *fdbv1beta2.FoundationDBStatus, expected map[string]fdbv1beta2.None) {
Expect(parseIncompatibleConnections(logr.Discard(), status)).To(Equal(expected))
Expect(parseIncompatibleConnections(logr.Discard(), status, "")).To(Equal(expected))
},
Entry("empty incompatible map",
&fdbv1beta2.FoundationDBStatus{
Expand Down
1 change: 0 additions & 1 deletion internal/locality/locality.go
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,6 @@ func CheckCoordinatorValidity(logger logr.Logger, cluster *fdbv1beta2.Foundation
coordinatorLocalities[field] = make(map[string]int)
}

// we do not want to isolate to single DC as this needs to examine all coordinators in the cluster
for _, process := range status.Cluster.Processes {
processGroupID := process.Locality[fdbv1beta2.FDBLocalityInstanceIDKey]

Expand Down
1 change: 0 additions & 1 deletion internal/maintenance/maintenance.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ func GetMaintenanceInformation(logger logr.Logger, status *fdbv1beta2.Foundation
return nil, nil, nil
}

// should be cross-dc since we presumedly want maintenance info across DCs
for _, process := range status.Cluster.Processes {
// Only storage processes are affected by the maintenance mode.
if process.ProcessClass != fdbv1beta2.ProcessClassStorage {
Expand Down
1 change: 0 additions & 1 deletion kubectl-fdb/cmd/analyze.go
Original file line number Diff line number Diff line change
Expand Up @@ -532,7 +532,6 @@ func analyzeStatusInternal(cmd *cobra.Command, restConfig *rest.Config, clientSe
var foundIssues bool

processesWithError := make([]string, 0)
// should be cross-DC as analyze uses fdbcli cross-DC
for _, process := range status.Cluster.Processes {
if len(process.Messages) == 0 {
continue
Expand Down
1 change: 0 additions & 1 deletion kubectl-fdb/cmd/exclusion_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,6 @@ func getExclusionStatus(cmd *cobra.Command, restConfig *rest.Config, kubeClient

var ongoingExclusions []exclusionResult
timestamp := time.Now()
// seems like it could be useful to have cross-DC info output here
for _, process := range status.Cluster.Processes {
if !process.Excluded {
continue
Expand Down
3 changes: 0 additions & 3 deletions pkg/fdbstatus/status_checks.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,6 @@ func getRemainingAndExcludedFromStatus(logger logr.Logger, status *fdbv1beta2.Fo
}

// Check in the status output which processes are already marked for exclusion in the cluster
// this should be cross-DC info bc the totals are all cluster-wide
for _, process := range status.Cluster.Processes {
processAddresses := []string{
fmt.Sprintf("%s:%s", fdbv1beta2.FDBLocalityExclusionPrefix, process.Locality[fdbv1beta2.FDBLocalityInstanceIDKey]),
Expand Down Expand Up @@ -259,7 +258,6 @@ func GetExclusions(status *fdbv1beta2.FoundationDBStatus) ([]fdbv1beta2.ProcessA
func GetCoordinatorsFromStatus(status *fdbv1beta2.FoundationDBStatus) map[string]fdbv1beta2.None {
coordinators := make(map[string]fdbv1beta2.None)

// should be cross-DC (nature of function getting coordinators)
for _, pInfo := range status.Cluster.Processes {
for _, roleInfo := range pInfo.Roles {
if roleInfo.Role != string(fdbv1beta2.ProcessRoleCoordinator) {
Expand Down Expand Up @@ -294,7 +292,6 @@ func GetMinimumUptimeAndAddressMap(logger logr.Logger, cluster *fdbv1beta2.Found
minimumUptime = status.Cluster.RecoveryState.SecondsSinceLastRecovered
}

// this is fine since we presumedly want a cross-DC view like SecondsSinceLastRecovered?
for _, process := range status.Cluster.Processes {
// Ignore tester processes for this check
if process.ProcessClass == fdbv1beta2.ProcessClassTest {
Expand Down

0 comments on commit 73efaf7

Please sign in to comment.