Skip to content

Commit

Permalink
Exclude network diagnostics pods from the status manager
Browse files Browse the repository at this point in the history
Introduce a way to exclude a resource from the status manger
and use it for the network diagnostics daemonset and deployment.

Signed-off-by: Patryk Diak <pdiak@redhat.com>
  • Loading branch information
kyrtapz committed Apr 16, 2024
1 parent 0c457c9 commit c8a491b
Show file tree
Hide file tree
Showing 5 changed files with 96 additions and 14 deletions.
2 changes: 2 additions & 0 deletions bindata/network-diagnostics/network-check-source.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ metadata:
pod network connectivity checks
release.openshift.io/version: "{{.ReleaseVersion}}"
networkoperator.openshift.io/non-critical: ""
labels:
networkoperator.openshift.io/generates-operator-status: ""
spec:
replicas: 1
selector:
Expand Down
2 changes: 2 additions & 0 deletions bindata/network-diagnostics/network-check-target.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ metadata:
a dummy app to be checked by network-check-source pod
release.openshift.io/version: "{{.ReleaseVersion}}"
networkoperator.openshift.io/non-critical: ""
labels:
networkoperator.openshift.io/generates-operator-status: ""
spec:
selector:
matchLabels:
Expand Down
17 changes: 10 additions & 7 deletions pkg/controller/operconfig/operconfig_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -424,14 +424,17 @@ func (r *ReconcileOperConfig) Reconcile(ctx context.Context, request reconcile.R
l = map[string]string{}
}

// In HyperShift use the infrastructure name to differentiate between resources deployed by the management cluster CNO and CNO deployed in the hosted clusters control plane namespace
// Without that the CNO running against the management cluster would pick the resources rendered by the hosted cluster CNO
if hcpCfg.Enabled {
l[names.GenerateStatusLabel] = bootstrapResult.Infra.InfraName
} else {
l[names.GenerateStatusLabel] = names.StandAloneClusterName
// Resources with GenerateStatusLabel set to "" are not meant to generate status
if v, exists := l[names.GenerateStatusLabel]; !exists || v != "" {
// In HyperShift use the infrastructure name to differentiate between resources deployed by the management cluster CNO and CNO deployed in the hosted clusters control plane namespace
// Without that the CNO running against the management cluster would pick the resources rendered by the hosted cluster CNO
if hcpCfg.Enabled {
l[names.GenerateStatusLabel] = bootstrapResult.Infra.InfraName
} else {
l[names.GenerateStatusLabel] = names.StandAloneClusterName
}
obj.SetLabels(l)
}
obj.SetLabels(l)
}
restMapping, err := r.mapper.RESTMapping(obj.GroupVersionKind().GroupKind())
if err != nil {
Expand Down
88 changes: 81 additions & 7 deletions pkg/controller/statusmanager/status_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func setStatus(t *testing.T, client cnoclient.Client, obj crclient.Object) {
}

// sl: labels that all status-havers have
var sl map[string]string = map[string]string{names.GenerateStatusLabel: ""}
var sl map[string]string = map[string]string{names.GenerateStatusLabel: names.StandAloneClusterName}

// Tests that the parts of newConditions that are set match what's in oldConditions (but
// doesn't look at anything else in oldConditions)
Expand Down Expand Up @@ -115,7 +115,7 @@ func conditionsEqual(oldConditions, newConditions []operv1.OperatorCondition) bo

func TestStatusManager_set(t *testing.T) {
client := fake.NewFakeClient()
status := New(client, "testing", "")
status := New(client, "testing", names.StandAloneClusterName)

// No operator config yet; should reflect this in the cluster operator
status.set(false)
Expand Down Expand Up @@ -255,7 +255,7 @@ func TestStatusManager_set(t *testing.T) {

func TestStatusManagerSetDegraded(t *testing.T) {
client := fake.NewFakeClient()
status := New(client, "testing", "")
status := New(client, "testing", names.StandAloneClusterName)

_, err := getOC(client)
if !errors.IsNotFound(err) {
Expand Down Expand Up @@ -347,7 +347,7 @@ func TestStatusManagerSetDegraded(t *testing.T) {

func TestStatusManagerSetFromDaemonSets(t *testing.T) {
client := fake.NewFakeClient()
status := New(client, "testing", "")
status := New(client, "testing", names.StandAloneClusterName)
setFakeListers(status)
no := &operv1.Network{ObjectMeta: metav1.ObjectMeta{Name: names.OPERATOR_CONFIG}}
set(t, client, no)
Expand Down Expand Up @@ -949,7 +949,7 @@ func TestStatusManagerSetFromDaemonSets(t *testing.T) {

func TestStatusManagerSetFromDeployments(t *testing.T) {
client := fake.NewFakeClient()
status := New(client, "testing", "")
status := New(client, "testing", names.StandAloneClusterName)
setFakeListers(status)
no := &operv1.Network{ObjectMeta: metav1.ObjectMeta{Name: names.OPERATOR_CONFIG}}
set(t, client, no)
Expand Down Expand Up @@ -1288,7 +1288,7 @@ func setLastPodState(t *testing.T, client cnoclient.Client, name string, ps podS

func TestStatusManagerCheckCrashLoopBackOffPods(t *testing.T) {
client := fake.NewFakeClient()
status := New(client, "testing", "")
status := New(client, "testing", names.StandAloneClusterName)
setFakeListers(status)
no := &operv1.Network{ObjectMeta: metav1.ObjectMeta{Name: names.OPERATOR_CONFIG}}
set(t, client, no)
Expand Down Expand Up @@ -1530,7 +1530,7 @@ func TestStatusManagerHyperShift(t *testing.T) {

// mgmt status
mgmtClient := fake.NewFakeClient()
mgmtStatus := New(mgmtClient, "testing", "")
mgmtStatus := New(mgmtClient, "testing", names.StandAloneClusterName)
setFakeListers(mgmtStatus)
no := &operv1.Network{ObjectMeta: metav1.ObjectMeta{Name: names.OPERATOR_CONFIG}}
set(t, mgmtClient, no)
Expand Down Expand Up @@ -1587,3 +1587,77 @@ func TestStatusManagerHyperShift(t *testing.T) {
t.Fatalf("unexpected Status.Conditions: %#v", oc.Status.Conditions)
}
}

func TestStatusManagerSetFromDeploymentsWithExcluded(t *testing.T) {
client := fake.NewFakeClient()
status := New(client, "testing", names.StandAloneClusterName)
setFakeListers(status)
no := &operv1.Network{ObjectMeta: metav1.ObjectMeta{Name: names.OPERATOR_CONFIG}}
set(t, client, no)

status.SetFromPods()

co, oc, err := getStatuses(client, "testing")
if err != nil {
t.Fatalf("error getting ClusterOperator: %v", err)
}
if !conditionsInclude(oc.Status.Conditions, []operv1.OperatorCondition{
{
Type: operv1.OperatorStatusTypeProgressing,
Status: operv1.ConditionTrue,
Reason: "Deploying",
},
}) {
t.Fatalf("unexpected Status.Conditions: %#v", oc.Status.Conditions)
}
if len(co.Status.Versions) > 0 {
t.Fatalf("Status.Versions unexpectedly already set: %#v", co.Status.Versions)
}

// Create minimal tracked Deployment
depA := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Namespace: "one", Name: "alpha", Labels: sl}, Status: appsv1.DeploymentStatus{
Replicas: 1,
UpdatedReplicas: 1,
ReadyReplicas: 1,
AvailableReplicas: 1,
}}
set(t, client, depA)

// Create another deployment that is not ready but doesn't affect the status since it is excluded(GenerateStatusLabel is value is empty)
depB := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Namespace: "one", Name: "beta", Labels: map[string]string{names.GenerateStatusLabel: ""}}, Status: appsv1.DeploymentStatus{
Replicas: 10,
UnavailableReplicas: 10,
}}
set(t, client, depB)

status.SetFromPods()

co, oc, err = getStatuses(client, "testing")
if err != nil {
t.Fatalf("error getting ClusterOperator: %v", err)
}
if !conditionsInclude(oc.Status.Conditions, []operv1.OperatorCondition{
{
Type: operv1.OperatorStatusTypeDegraded,
Status: operv1.ConditionFalse,
},
{
Type: operv1.OperatorStatusTypeProgressing,
Status: operv1.ConditionFalse,
},
{
Type: operv1.OperatorStatusTypeUpgradeable,
Status: operv1.ConditionTrue,
},
{
Type: operv1.OperatorStatusTypeAvailable,
Status: operv1.ConditionTrue,
},
}) {
t.Fatalf("unexpected Status.Conditions: %#v", oc.Status.Conditions)
}
if len(co.Status.Versions) != 1 {
t.Fatalf("unexpected Status.Versions: %#v", co.Status.Versions)
}

}
1 change: 1 addition & 0 deletions pkg/names/names.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ const NonCriticalAnnotation = "networkoperator.openshift.io/non-critical"
// Currently, this is looked for on Deployments, DaemonSets, and StatefulSets.
// Its value reflects which cluster the resource belongs to. This helps avoid an overlap
// in Hypershift where there can be multiple CNO instances running in the management cluster.
// If the value is empty, the resource is not going to be tracked by StatusController.
const GenerateStatusLabel = "networkoperator.openshift.io/generates-operator-status"

// StandAloneClusterName is a value used for GenerateStatusLabel label when running in non-Hypershift environments
Expand Down

0 comments on commit c8a491b

Please sign in to comment.