KO-285: Allow Operator operations with failed pods #257
Conversation
api/v1/aerospikecluster_types.go
Outdated
| @@ -72,7 +72,13 @@ type AerospikeClusterSpec struct { //nolint:govet // for readability | |||
| // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Seeds Finder Services" | |||
| SeedsFinderServices SeedsFinderServices `json:"seedsFinderServices,omitempty"` | |||
| // RosterNodeBlockList is a list of blocked nodeIDs from roster in a strong-consistency setup | |||
| // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Roster NodeB lockList" | |||
There was a problem hiding this comment.
nit: check for extra space in "Roster NodeB lockList" after 'B'
| @@ -60,6 +67,10 @@ spec: | |||
| cluster. Pods will be deployed in given racks based on given configuration | |||
| displayName: Rack Config | |||
| path: rackConfig | |||
| - description: RosterNodeBlockList is a list of blocked nodeIDs from roster | |||
| in a strong-consistency setup | |||
| displayName: Roster NodeB lockList | |||
| @@ -18,6 +18,7 @@ import ( | |||
| "time" | |||
|
|
|||
| corev1 "k8s.io/api/core/v1" | |||
| "k8s.io/apimachinery/pkg/util/sets" | |||
There was a problem hiding this comment.
we can skip changing existing implementation of sets, but for new use-case can we use golang-set? Or may be in some files we are already imported apimachinery sets and you don't want to import both, in that case it's fine.
There was a problem hiding this comment.
golang-set is only used in SC file for management-lib changes.
As discussed, we won't change existing code to golang-set in this PR, so adding new changes with golang-set was causing confusion. So kept everything on apimachinery set pkg and will move to golang-set in one go.
controllers/aero_info_calls.go
Outdated
| @@ -34,19 +35,19 @@ import ( | |||
| // The ignorablePods list should be a list of failed or pending pods that are going to be | |||
There was a problem hiding this comment.
change comment as we have changed the variable name.
test/cluster_test.go
Outdated
| By("Scale up") | ||
| aeroCluster, err = getCluster(k8sClient, ctx, clusterNamespacedName) | ||
| Expect(err).ToNot(HaveOccurred()) | ||
| aeroCluster.Spec.IgnorePodList = []string{ignorePodName} |
There was a problem hiding this comment.
any reason of setting it again after setting it at line 179? AFAIK only user can remove the name from IgnorePodList right?
There was a problem hiding this comment.
Not required.
test/cluster_test.go
Outdated
| @@ -124,6 +129,92 @@ func ScaleDownWithMigrateFillDelay(ctx goctx.Context) { | |||
| ) | |||
| } | |||
|
|
|||
| func clusterWithIgnorePodList(ctx goctx.Context) { | |||
There was a problem hiding this comment.
can we add a test after removing the name from list and make that pod running to check if updated config is correctly reflected on that pod as well?
api/v1/aerospikecluster_types.go
Outdated
| RosterNodeBlockList []string `json:"rosterNodeBlockList,omitempty"` | ||
| // IgnorePodList is the list of pods which are ignored by the operator while checking the cluster stability and |
There was a problem hiding this comment.
// IgnorePodList is a list of pods that the operator will ignore while assessing cluster stability.
// Pods specified in this list are not considered part of the cluster. This is particularly useful when
// there are failed pods and the operator needs to perform certain operations on the cluster. Note that
// running pods included in this list will not be ignored.
abhishekdwivedi3060
force-pushed
the
KO-285
branch
from
ba3bb6e to
5850431
Compare
abhishekdwivedi3060
force-pushed
the
KO-285
branch
from
586066c to
ba63462
Compare
| for idx := range pods { | ||
| pod := pods[idx] | ||
| if ignorablePodNames.Has(pod.Name) { |
There was a problem hiding this comment.
we calls getFailedAndActivePods at 4 places, at 2 places we make sure that there are no ignorable pods present in pods list passed to this function, and in other two places from reconcileRacks(), there are chances of having ignorable pods in pods list. Can we write it better and remove check if ignorablePodNames.Has(pod.Name) { from getFailedAndActivePods if possible.
No description provided.