Skip to content

Commit 914169b

Browse files
feat: patch CR status via the Kubernetes client, not kubectl
1 parent 11bacdc commit 914169b

6 files changed

Lines changed: 254 additions & 122 deletions

File tree

‎python/openstack-sync/openstack_sync/hooks/common.py‎

Lines changed: 95 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
"""Generic shell-operator hook utilities shared across all hooks.
22
3-
Provides binding context I/O and status patching via kubectl.
3+
Provides binding context I/O and CR status patching.
44
"""
55

66
from __future__ import annotations
@@ -9,10 +9,14 @@
99
import json
1010
import logging
1111
import os
12-
import subprocess
1312
import sys
1413
from typing import Any
1514

15+
from kubernetes import client as k8s_client
16+
from kubernetes.client.exceptions import ApiException
17+
18+
from openstack_sync.utils import load_kubernetes_config
19+
1620
LOG = logging.getLogger(__name__)
1721

1822

@@ -181,27 +185,74 @@ def _status_is_current(
181185
return True
182186

183187

188+
#: Memoised CustomObjectsApi, so one config load serves every patch in a run.
189+
_custom_objects_api: Any = None
190+
191+
192+
def _customobjects_api() -> Any:
193+
"""Return the shared ``CustomObjectsApi``, configuring the client once."""
194+
global _custom_objects_api
195+
if _custom_objects_api is None:
196+
load_kubernetes_config()
197+
_custom_objects_api = k8s_client.CustomObjectsApi()
198+
return _custom_objects_api
199+
200+
201+
def crd_request_target(crd_api_version: str, crd_resource: str) -> tuple[str, str, str]:
202+
"""Return ``(group, version, plural)`` for addressing a CR.
203+
204+
Derived from the two environment values the chart already injects, rather
205+
than from new ones: ``<prefix>_CRD_API_VERSION`` is ``<group>/<version>``
206+
and ``<prefix>_CRD_RESOURCE`` is ``<plural>.<group>``, which between them
207+
carry everything the API needs.
208+
209+
Raises:
210+
ValueError: When either value is not in the expected shape.
211+
"""
212+
group, _, version = crd_api_version.partition("/")
213+
plural = crd_resource.partition(".")[0]
214+
if not group or not version or not plural:
215+
raise ValueError(
216+
f"cannot derive a CRD request target from "
217+
f"api_version={crd_api_version!r} and resource={crd_resource!r}"
218+
)
219+
return group, version, plural
220+
221+
222+
def _api_error_detail(exc: ApiException, max_body: int = 512) -> str:
223+
"""Return a one-line description of *exc* for a log message."""
224+
body = str(exc.body or "").strip().replace("\n", " ")
225+
detail = f"HTTP {exc.status} {exc.reason or ''}".strip()
226+
if not body:
227+
return detail
228+
return f"{detail}: {truncate_message(body, max_body)}"
229+
230+
184231
def patch_resource_status(
185232
*,
186233
name: str,
187234
namespace: str | None,
188235
generation: int | None,
189236
sync_status: str,
190237
message: str,
238+
crd_api_version: str,
191239
crd_resource: str,
192240
crd_kind: str,
193241
status_enabled: bool,
194242
current_status: dict[str, Any] | None = None,
195243
) -> None:
196-
"""Patch the status subresource of a CR via kubectl.
244+
"""Patch the status subresource of a CR.
197245
198246
Args:
199247
name: CR metadata.name.
200-
namespace: CR metadata.namespace (optional).
248+
namespace: CR metadata.namespace. Required to address the object; the
249+
patch is skipped with a warning when it is absent.
201250
generation: CR metadata.generation for observedGeneration (optional).
202251
sync_status: One of ``"Synced"`` or ``"Failed"``.
203252
message: Human-readable detail for the status message.
204-
crd_resource: Fully-qualified CRD resource name for kubectl (e.g.
253+
crd_api_version: CRD API version as ``<group>/<version>`` (e.g.
254+
``neutron.understack.rackspace.net/v1alpha1``).
255+
crd_resource: Fully-qualified CRD resource name (e.g.
205256
``neutronrouterflavors.neutron.understack.rackspace.net``).
206257
crd_kind: CRD kind used in log messages (e.g. ``NeutronRouterFlavor``).
207258
status_enabled: When False the function returns immediately.
@@ -231,32 +282,46 @@ def patch_resource_status(
231282
if generation is not None:
232283
status["observedGeneration"] = generation
233284

234-
command = [
235-
"kubectl",
236-
"patch",
237-
crd_resource,
238-
name,
239-
"--type",
240-
"merge",
241-
"--subresource",
242-
"status",
243-
"-p",
244-
json.dumps({"status": status}, sort_keys=True),
245-
]
246-
if namespace:
247-
command.extend(["-n", namespace])
285+
if not namespace:
286+
LOG.warning(
287+
"unable to patch %s status for %s; no namespace to address it in",
288+
crd_kind,
289+
name,
290+
)
291+
return
248292

249293
try:
250-
result = subprocess.run( # noqa: S603,S607
251-
command,
252-
capture_output=True,
253-
check=False,
254-
text=True,
255-
)
256-
except FileNotFoundError:
257-
LOG.warning("kubectl not found; unable to patch %s status", crd_kind)
294+
group, version, plural = crd_request_target(crd_api_version, crd_resource)
295+
except ValueError as exc:
296+
LOG.warning("unable to patch %s status for %s: %s", crd_kind, name, exc)
258297
return
259298

260-
if result.returncode != 0:
261-
error = (result.stderr or result.stdout or "unknown error").strip()
262-
LOG.warning("failed to patch %s status for %s: %s", crd_kind, name, error)
299+
# The client's default content type here is merge-patch+json, which is what
300+
# `kubectl patch --type merge --subresource status` sent before this moved
301+
# off kubectl, so the request on the wire is unchanged.
302+
try:
303+
_customobjects_api().patch_namespaced_custom_object_status(
304+
group=group,
305+
version=version,
306+
namespace=namespace,
307+
plural=plural,
308+
name=name,
309+
body={"status": status},
310+
)
311+
except ApiException as exc:
312+
LOG.warning(
313+
"failed to patch %s status for %s: %s",
314+
crd_kind,
315+
name,
316+
_api_error_detail(exc),
317+
)
318+
except Exception as exc: # noqa: BLE001
319+
# Config load and transport failures reach here. A status patch is
320+
# reporting, never the work itself, so it must not fail the reconcile.
321+
LOG.warning(
322+
"failed to patch %s status for %s: %s: %s",
323+
crd_kind,
324+
name,
325+
type(exc).__name__,
326+
exc,
327+
)

‎python/openstack-sync/openstack_sync/hooks/framework.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -502,6 +502,7 @@ def _patch_status(
502502
generation=resource.generation,
503503
sync_status=sync_status,
504504
message=message,
505+
crd_api_version=config.crd_api_version,
505506
crd_resource=config.crd_resource,
506507
crd_kind=config.crd_kind,
507508
status_enabled=config.status_enabled,

‎python/openstack-sync/openstack_sync/utils.py‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,20 @@
1717
from openstack.config import loader as os_config_loader
1818

1919

20-
def read_secret_key(secret_name: str, secret_key: str, namespace: str) -> str:
21-
"""Read a single key from a Kubernetes Secret and return its decoded value.
20+
def load_kubernetes_config() -> None:
21+
"""Configure the Kubernetes client for wherever this is running.
2222
23-
Configures the client automatically:
2423
- Inside a cluster: uses the pod's service-account token.
2524
- Outside a cluster: falls back to the local kubeconfig (development).
25+
"""
26+
try:
27+
k8s_config.load_incluster_config()
28+
except k8s_config.ConfigException:
29+
k8s_config.load_kube_config()
30+
31+
32+
def read_secret_key(secret_name: str, secret_key: str, namespace: str) -> str:
33+
"""Read a single key from a Kubernetes Secret and return its decoded value.
2634
2735
Args:
2836
secret_name: Name of the Kubernetes Secret to read.
@@ -35,10 +43,7 @@ def read_secret_key(secret_name: str, secret_key: str, namespace: str) -> str:
3543
Raises:
3644
KeyError: When ``secret_key`` is not present in the Secret's data.
3745
"""
38-
try:
39-
k8s_config.load_incluster_config()
40-
except k8s_config.ConfigException:
41-
k8s_config.load_kube_config()
46+
load_kubernetes_config()
4247

4348
v1 = client.CoreV1Api()
4449
secret = v1.read_namespaced_secret(name=secret_name, namespace=namespace)
@@ -89,7 +94,7 @@ def get_openstack_connection(secret_name: str, cloud_name: str) -> Any:
8994
# openstack.connect(cloud=name) resolves the named cloud from files on
9095
# disk, which fails inside a container with no clouds.yaml present.
9196
# Instead, parse the YAML from the Secret and inject it directly into
92-
# the SDK config loader — no filesystem writes required.
97+
# the SDK config loader -- no filesystem writes required.
9398
loader = os_config_loader.OpenStackConfig(
9499
config_files=[],
95100
vendor_files=[],

‎python/openstack-sync/tests/test_framework.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -767,8 +767,8 @@ def test_run_sync_connects_with_each_resources_own_credentials():
767767
def test_run_sync_forwards_crd_identity_and_current_status_to_the_patch():
768768
"""Forward everything patch_resource_status needs.
769769
770-
The CRD identity targets kubectl, and the current status decides whether the
771-
patch can be skipped.
770+
The CRD identity addresses the object on the API, and the current status
771+
decides whether the patch can be skipped.
772772
"""
773773
config = make_hook_config(status_enabled=True)
774774
plugin = StubPlugin(config)

0 commit comments

Comments
 (0)