Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions src/in_cluster_checks/domains/cluster_overview_domain.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
"""
Cluster overview rule domain.

Collects informational, read-only snapshots describing what the cluster is
(architecture, versions, stack) rather than validating pass/fail conditions.
"""

from typing import List

from in_cluster_checks.core.domain import RuleDomain
from in_cluster_checks.rules.cluster_overview.architecture_overview import ClusterArchitectureOverview


class ClusterOverviewDomain(RuleDomain):
"""
Cluster overview domain.

Groups informational collection rules that describe the cluster's
architecture: identity, topology, network, storage, and platform services.
"""

def domain_name(self) -> str:
"""Get domain name."""
return "cluster_overview"

def get_rule_classes(self) -> List[type]:
"""
Get list of cluster overview rules to run.

Returns:
List of Rule classes
"""
return [
ClusterArchitectureOverview,
]
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,215 @@
"""
Cluster architecture overview collection.

Collects a structured, read-only snapshot of what the cluster IS — identity,
topology, network, storage, identity providers, and installed operators —
via the cluster API. Contributed from the ocp-analyzer project.
"""

from in_cluster_checks.core.rule import OrchestratorRule
from in_cluster_checks.core.rule_result import RuleResult
from in_cluster_checks.utils.enums import Objectives
from in_cluster_checks.utils.parsing_utils import parse_json

VERSION_HISTORY_LIMIT = 12


class ClusterArchitectureOverview(OrchestratorRule):
"""Collect a read-only architecture overview of the cluster.

Gathers cluster identity (version, channel, platform, base domain),
topology (nodes by role, control-plane topology), network (CNI type,
cluster/service CIDRs, MTU), storage classes, identity providers, and
installed operators into a structured INFO result.

The ClusterVersion resource is required; every other section degrades
gracefully (e.g. RBAC denied, resource absent) so a partial overview
is still reported instead of failing the whole rule.
"""

objective_hosts = [Objectives.ORCHESTRATOR]
unique_name = "cluster_architecture_overview"
title = "Cluster architecture overview"
supported_profiles = {"general"}
links = [
"https://docs.openshift.com/container-platform/latest/architecture/architecture.html",
]
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Comment thread
coderabbitai[bot] marked this conversation as resolved.

def run_rule(self) -> RuleResult:
"""Collect all overview sections and return them as a structured INFO result."""
infrastructure = self._get_resource(["infrastructure", "cluster"]) or {}

overview = {
"cluster_identity": self._collect_cluster_identity(infrastructure),
"topology": self._collect_topology(infrastructure),
"network": self._collect_network(),
"storage": self._collect_storage(),
"identity_providers": self._collect_identity_providers(),
"operators": self._collect_operators(),
}

return RuleResult.info(self._build_summary(overview), system_info=overview)

def _get_resource(self, resource_args: list, required: bool = False) -> dict | None:
"""Fetch a cluster resource as parsed JSON.

Args:
resource_args: Arguments for `oc get` (e.g. ["network.config", "cluster"])
required: If True, a failed command raises UnExpectedSystemOutput
(rule becomes SKIP); otherwise None is returned

Returns:
Parsed resource dict, or None if unavailable and not required
"""
args = [*resource_args, "-o", "json"]
return_code, output, _ = self.oc_api.run_oc_command("get", args, timeout=45, raise_on_error=required)
if return_code != 0:
return None
return parse_json(output, f"oc get {' '.join(args)}", self.get_host_ip())
Comment on lines +56 to +68

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The project convention is to use self.oc_api.select_resources() instead of run_oc_command + parse_json(). It returns parsed Python objects (accessed via .model), handles timeouts, logs commands, and provides debug output — all the things this helper reimplements manually.

This is a larger refactor than just swapping _get_resource, since:

  • select_resources() returns openshift-client objects, so all _collect_* methods would change from .get("spec", {}) to .model.spec
  • For list resources (storageclass, subscriptions), it returns a flat list — not a {"items": [...]} dict — so storage_classes.get("items", []) becomes direct iteration
  • Error handling differs: select_resources() raises OpenShiftPythonException on RBAC denial instead of returning a non-zero return code. The _get_resource wrapper would need a try/except for non-required resources
  • Some resources already have dedicated helpers (e.g. self.oc_api.get_all_nodes())

Example of what _get_resource could look like wrapping select_resources:

def _get_resource(self, resource_type, single=True, all_namespaces=False, required=False):
    try:
        return self.oc_api.select_resources(resource_type, single=single, all_namespaces=all_namespaces, timeout=45)
    except Exception:
        if required:
            raise
        return None

And callers would use .model access:

# e.g. in _collect_network:
network_config = self._get_resource("network.config/cluster")
if not network_config:
    return {}
return {
    "network_type": network_config.model.status.get("networkType"),
    ...
}

It's more work upfront but keeps the rule consistent with the rest of the codebase and drops the parse_json dependency.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rule_file="src/in_cluster_checks/rules/cluster_overview/architecture_overview.py"

ast-grep outline "$rule_file" --items all --view expanded
printf '\n--- select_resources definitions and usages ---\n'
rg -n -C 4 '\bselect_resources\s*\(' src tests
printf '\n--- OpenShiftPythonException references ---\n'
rg -n -C 3 '\bOpenShiftPythonException\b' src tests
printf '\n--- complete rule ---\n'
cat -n "$rule_file"

Length of output: 37212


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- OcApiUtils.select_resources ---'
sed -n '23,145p' src/in_cluster_checks/utils/oc_api_utils.py

printf '%s\n' '--- framework exception handling ---'
rg -n -C 5 'UnExpectedSystemOutput|OpenShiftPythonException' src/in_cluster_checks/core src/in_cluster_checks

printf '%s\n' '--- existing narrow exception conversion patterns ---'
rg -n -C 8 'except oc\.OpenShiftPythonException|except OpenShiftPythonException' src/in_cluster_checks/rules src/in_cluster_checks/utils

printf '%s\n' '--- architecture overview tests ---'
sed -n '1,280p' tests/rules/cluster_overview/test_architecture_overview.py

Length of output: 124403


@sprizend-rh, verified. This refactor is valid.

select_resources() provides the command log, timeout, and debug behavior that this rule currently duplicates. The rule should remove parse_json and use resource objects or .as_dict() consistently.

One exception detail is important. select_resources() propagates OpenShiftPythonException, but the runner converts only UnExpectedSystemOutput to the required skip result. Therefore, _get_resource() must:

  • Catch only OpenShiftPythonException.
  • Return None for an optional resource.
  • Convert the exception to UnExpectedSystemOutput for required clusterversion/version.
  • Not catch broad Exception.

The refactor must also update list handling for nodes, storage classes, and subscriptions. It should use get_all_nodes() for nodes. The tests must replace run_oc_command() output fixtures with select_resources() object/list fixtures and OpenShiftPythonException cases.


✏️ Learnings added
Learnt from: sprizend-rh
Repo: RedHatInsights/incluster-checks PR: 95
File: src/in_cluster_checks/rules/cluster_overview/architecture_overview.py:56-68
Timestamp: 2026-08-10T09:28:39.206Z
Learning: For Python in-cluster rules in `src/in_cluster_checks/rules`, use `self.oc_api.select_resources()` for Kubernetes and OpenShift resource queries instead of `run_oc_command()` followed by JSON parsing. `select_resources()` provides command logging, timeouts, and debug output; it returns resource objects for `single=True` and lists of resource objects otherwise. When an optional resource query must degrade gracefully, catch only `openshift_client.OpenShiftPythonException`. When a required query fails, convert that exception to `UnExpectedSystemOutput` so the framework reports the rule as SKIP.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

You are interacting with an AI system.


def _collect_cluster_identity(self, infrastructure: dict) -> dict:
"""Collect version, channel, cluster ID, platform, and base domain."""
cluster_version = self._get_resource(["clusterversion", "version"], required=True)
spec = cluster_version.get("spec", {})
status = cluster_version.get("status", {})
identity = {
"version": status.get("desired", {}).get("version"),
"channel": spec.get("channel"),
"cluster_id": spec.get("clusterID"),
"version_history": [entry.get("version") for entry in status.get("history", [])[:VERSION_HISTORY_LIMIT]],
}

infra_status = infrastructure.get("status", {})
identity["platform"] = infra_status.get("platformStatus", {}).get("type")
identity["infrastructure_name"] = infra_status.get("infrastructureName")
identity["api_server_url"] = infra_status.get("apiServerURL")

dns_config = self._get_resource(["dns.config", "cluster"])
if dns_config:
identity["base_domain"] = dns_config.get("spec", {}).get("baseDomain")

return identity

def _collect_topology(self, infrastructure: dict) -> dict:
"""Collect node counts by role, control-plane topology, and node software versions."""
infra_status = infrastructure.get("status", {})
topology = {
"control_plane_topology": infra_status.get("controlPlaneTopology"),
"infrastructure_topology": infra_status.get("infrastructureTopology"),
}

nodes = self._get_resource(["nodes"])
if not nodes:
return topology

items = nodes.get("items", [])
nodes_by_role = {}
kubelet_versions = set()
os_images = set()
for node in items:
for role in self._get_node_roles(node):
nodes_by_role[role] = nodes_by_role.get(role, 0) + 1
node_info = node.get("status", {}).get("nodeInfo", {})
if node_info.get("kubeletVersion"):
kubelet_versions.add(node_info["kubeletVersion"])
if node_info.get("osImage"):
os_images.add(node_info["osImage"])

topology["node_count"] = len(items)
topology["nodes_by_role"] = nodes_by_role
topology["kubelet_versions"] = sorted(kubelet_versions)
topology["os_images"] = sorted(os_images)
return topology

@staticmethod
def _get_node_roles(node: dict) -> list:
"""Extract role names from a node's node-role.kubernetes.io/* labels."""
labels = node.get("metadata", {}).get("labels", {})
roles = [label.split("/", 1)[1] for label in labels if label.startswith("node-role.kubernetes.io/")]
return sorted(roles) if roles else ["unknown"]
Comment on lines +124 to +129

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This duplicates role-extraction logic from NodeExecutorFactory._get_role_labels_string() (same label prefix parsing and sorting). Since OrchestratorRule has access to self._node_executors, the topology section can read roles from executor.node_labels — the same pattern NodeResourcesCollector._extract_roles() uses in resources_utilization.py.

You'd still need get_all_nodes() for kubeletVersion/osImage (not stored on executors), but the role counting can come from the executors, which removes this static method entirely.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- candidate files ---'
fd -t f -a 'resources_utilization.py|node_executor.*\.py|node_executor_factory.*\.py|architecture_overview.py' . | sort

echo '--- matching definitions and uses ---'
rg -n -C 5 \
  'def _get_role_labels_string\b|def _extract_roles\b|_node_executors\b|node_labels\b|def get_all_nodes\b' \
  src tests || true

echo '--- outlines ---'
for file in \
  src/in_cluster_checks/rules/cluster_overview/architecture_overview.py \
  src/in_cluster_checks/utils/resources_utilization.py
do
  if [ -f "$file" ]; then
    ast-grep outline "$file" --items all
  fi
done

Length of output: 69138


@sprizend-rh, the executor labels use the same sorted role names. However, NodeExecutorFactory creates an executor only when a Node has an internal IP.

This rule obtains node_count from oc get nodes. If it obtains nodes_by_role from self._node_executors, a Node without an internal IP can be included in node_count but omitted from its role count.

I recommend retaining the API Node list as the source for both fields. Extract the common node-role.kubernetes.io/* parsing into a shared utility, then use that utility from NodeExecutorFactory and ClusterArchitectureOverview. This removes duplication without changing the overview snapshot semantics.


✏️ Learnings added
Learnt from: sprizend-rh
Repo: RedHatInsights/incluster-checks PR: 95
File: src/in_cluster_checks/rules/cluster_overview/architecture_overview.py:124-129
Timestamp: 2026-08-10T09:28:46.098Z
Learning: In `src/in_cluster_checks/core/executor_factory.py`, `NodeExecutorFactory` creates a `NodeExecutor` only when the Kubernetes Node has an internal IP. For cluster-wide inventory rules, `self._node_executors` can omit Nodes that `oc get nodes` returns, so the API Node list must remain the source for complete node counts and role aggregation.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

You are interacting with an AI system.


def _collect_network(self) -> dict:
"""Collect CNI type, cluster/service CIDRs, and MTU."""
network_config = self._get_resource(["network.config", "cluster"])
if not network_config:
return {}

status = network_config.get("status", {})
return {
"network_type": status.get("networkType"),
"cluster_network": [entry.get("cidr") for entry in status.get("clusterNetwork", [])],
"service_network": status.get("serviceNetwork", []),
"cluster_network_mtu": status.get("clusterNetworkMTU"),
}

def _collect_storage(self) -> dict:
"""Collect storage classes and which of them are default."""
storage_classes = self._get_resource(["storageclass"])
if not storage_classes:
return {}

classes = []
default_classes = []
for storage_class in storage_classes.get("items", []):
metadata = storage_class.get("metadata", {})
name = metadata.get("name")
annotations = metadata.get("annotations") or {}
is_default = annotations.get("storageclass.kubernetes.io/is-default-class") == "true"
classes.append(
{
"name": name,
"provisioner": storage_class.get("provisioner"),
"default": is_default,
}
)
if is_default:
default_classes.append(name)

return {"storage_classes": classes, "default_storage_classes": default_classes}

def _collect_identity_providers(self) -> list:
"""Collect configured identity provider names and types (no credentials)."""
oauth = self._get_resource(["oauth", "cluster"])
if not oauth:
return []

providers = oauth.get("spec", {}).get("identityProviders") or []
return [{"name": provider.get("name"), "type": provider.get("type")} for provider in providers]

def _collect_operators(self) -> list:
"""Collect installed operator subscriptions (name, namespace, channel, CSV)."""
subscriptions = self._get_resource(["subscriptions.operators.coreos.com", "--all-namespaces"])

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

self.oc_api.get_operator_subscriptions() already does exactly this — same resource type, same --all-namespaces, same parse_json. It's used by two rules in k8s_validations.py. You could call it directly instead of going through _get_resource:

def _collect_operators(self) -> list:
    subscriptions = self.oc_api.get_operator_subscriptions()
    # ... rest of the mapping logic

Note: get_operator_subscriptions() raises UnExpectedSystemOutput on failure (no graceful degradation), so you'd need a try/except if you want RBAC denial to degrade to an empty list.

if not subscriptions:
return []

operators = []
for item in subscriptions.get("items", []):
spec = item.get("spec", {})
metadata = item.get("metadata", {})
operators.append(
{
"name": spec.get("name") or metadata.get("name"),
"namespace": metadata.get("namespace"),
"channel": spec.get("channel"),
"installed_csv": item.get("status", {}).get("installedCSV"),
}
)

return sorted(operators, key=lambda operator: (operator["namespace"] or "", operator["name"] or ""))
Comment thread
coderabbitai[bot] marked this conversation as resolved.

@staticmethod
def _build_summary(overview: dict) -> str:
"""Build a one-line human-readable summary of the overview."""
identity = overview["cluster_identity"]
topology = overview["topology"]
network = overview["network"]

nodes_by_role = topology.get("nodes_by_role") or {}
roles_summary = ", ".join(f"{count}x {role}" for role, count in sorted(nodes_by_role.items()))
return (
f"OpenShift {identity.get('version') or 'unknown'} "
f"on {identity.get('platform') or 'unknown platform'} | "
f"{topology.get('node_count', 0)} nodes ({roles_summary or 'roles unknown'}) | "

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nit: when node_count == 1, this produces "1 nodes". Consider:

node_count = topology.get('node_count', 0)
f"{node_count} {'node' if node_count == 1 else 'nodes'} ({roles_summary or 'roles unknown'})"

f"CNI: {network.get('network_type') or 'unknown'} | "
f"operators: {len(overview['operators'])}"
)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
19 changes: 19 additions & 0 deletions tests/domains/test_cluster_overview_domain.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
"""Tests for cluster overview domain."""

from in_cluster_checks.domains.cluster_overview_domain import ClusterOverviewDomain
from in_cluster_checks.rules.cluster_overview.architecture_overview import ClusterArchitectureOverview


def test_cluster_overview_domain_name():
"""Test domain name."""
domain = ClusterOverviewDomain()
assert domain.domain_name() == "cluster_overview"


def test_cluster_overview_domain_rules():
"""Test domain returns correct rules."""
domain = ClusterOverviewDomain()
rules = domain.get_rule_classes()

assert len(rules) == 1
assert ClusterArchitectureOverview in rules
Empty file.
Loading
Loading