Skip to content

test(node-indexer): exercise ClairCore mapping download retries - #23436

Draft
vikin91 wants to merge 1 commit into
masterfrom
piotr/test-claircore-repo2cpe-retries
Draft

vikin91 wants to merge 1 commit into
masterfrom
piotr/test-claircore-repo2cpe-retries

Conversation

@vikin91

@vikin91 vikin91 commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Description

Pin the ClairCore fork's retry implementation and enable three retries for repo-to-CPE downloads. Keep the existing client and timeout, and cover recovery, exhaustion, permanent errors, and cancellation.

User-facing documentation

Testing and quality

  • the change is production ready: the change is GA, or otherwise the functionality is gated by a feature flag
  • CI results are inspected

Automated testing

  • added unit tests
  • added e2e tests
  • added regression tests
  • added compatibility tests
  • modified existing tests

How I validated my change

Verified on six RHCOS nodes manually on a cluster. Recovery produced 503,503,503,200 with approximately 1s, 2s, and 4s waits, followed by 572-package index reports reaching Sensor. Persistent 503 responses stopped after four requests; 404 responses were not retried.

Prerequisites: OpenShift with RHCOS nodes, kubectl, curl, Python 3, and a running Podman machine. Run from the StackRox checkout under test, with its CI images already published. These steps assume a fresh ACS deployment.

  1. Deploy the test image with node indexing and Scanner V4 enabled:
export KUBECONFIG=/path/to/test-cluster.kubeconfig

MAIN_IMAGE_TAG="$(make tag)" \
ROX_SCANNER_V4=true \
ROX_NODE_INDEX_ENABLED=true \
LOAD_BALANCER=route \
CLUSTER=node-retry-smoke \
./deploy/openshift/deploy.sh

kubectl -n stackrox rollout status deployment/sensor --timeout=5m
kubectl -n stackrox rollout status daemonset/collector --timeout=5m
  1. Create the temporary mapping server. Counters are separate for each endpoint and client IP. /recovery serves the real mapping after three failures; /exhausted always returns 503; /permanent always returns 404.
SMOKE_DIR="$(mktemp -d)"

curl -fsSL \
  https://security.access.redhat.com/data/metrics/repository-to-cpe.json \
  -o "$SMOKE_DIR/mapping.json"

python3 - "$SMOKE_DIR" <<'PY'
import gzip, pathlib, sys
root = pathlib.Path(sys.argv[1])
(root / "mapping.gz").write_bytes(
    gzip.compress((root / "mapping.json").read_bytes())
)
PY

cat > "$SMOKE_DIR/server.py" <<'PY'
import gzip
import json
import threading
import time
from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer
from pathlib import Path

mapping = gzip.decompress(Path("/fixture/mapping.gz").read_bytes())
counts = {}
lock = threading.Lock()

class Handler(BaseHTTPRequestHandler):
    def do_GET(self):
        key = (self.path, self.client_address[0])
        with lock:
            counts[key] = counts.get(key, 0) + 1
            attempt = counts[key]
        if self.path == "/exhausted":
            status = 503
        elif self.path == "/recovery" and attempt <= 3:
            status = 503
        elif self.path == "/permanent":
            status = 404
        else:
            status = 200
        body = mapping if status == 200 else b"injected mapping failure"
        print(json.dumps({
            "time": time.time(), "path": self.path,
            "client": self.client_address[0],
            "attempt": attempt, "status": status,
        }), flush=True)
        self.send_response(status)
        self.send_header("Content-Length", str(len(body)))
        self.end_headers()
        self.wfile.write(body)

    def log_message(self, *args):
        pass

ThreadingHTTPServer(("0.0.0.0", 8080), Handler).serve_forever()
PY

kubectl -n stackrox create configmap repo2cpe-smoke \
  --from-file=server.py="$SMOKE_DIR/server.py" \
  --from-file=mapping.gz="$SMOKE_DIR/mapping.gz" \
  --dry-run=client -o yaml | kubectl apply -f -

kubectl apply -f - <<'YAML'
apiVersion: apps/v1
kind: Deployment
metadata:
  name: repo2cpe-smoke
  namespace: stackrox
spec:
  replicas: 1
  selector:
    matchLabels:
      app: repo2cpe-smoke
  template:
    metadata:
      labels:
        app: repo2cpe-smoke
    spec:
      containers:
        - name: http
          image: docker.io/library/python:3.12-alpine
          command: ["python3", "/fixture/server.py"]
          ports:
            - containerPort: 8080
          volumeMounts:
            - name: fixture
              mountPath: /fixture
              readOnly: true
          resources:
            requests:
              cpu: 10m
              memory: 48Mi
            limits:
              memory: 128Mi
      volumes:
        - name: fixture
          configMap:
            name: repo2cpe-smoke
---
apiVersion: v1
kind: Service
metadata:
  name: repo2cpe-smoke
  namespace: stackrox
spec:
  selector:
    app: repo2cpe-smoke
  ports:
    - port: 8080
      targetPort: 8080
YAML

kubectl -n stackrox rollout status deployment/repo2cpe-smoke --timeout=3m

The Service provides repo2cpe-smoke.stackrox.svc:8080.

  1. Run all three scenarios through the actual node-scanning compliance containers. A one-second initial delay triggers scans promptly after each rollout.
EXPECTED_NODES="$(kubectl -n stackrox get ds collector \
  -o jsonpath='{.status.desiredNumberScheduled}')"

for scenario in recovery exhausted permanent; do
  CASE_START="$(date -u +%Y-%m-%dT%H:%M:%SZ)"

  kubectl -n stackrox set env ds/collector --containers=compliance \
    ROX_NODE_SCANNING_MAX_INITIAL_WAIT=1s \
    ROX_NODE_INDEX_MAPPING_URL="http://repo2cpe-smoke.stackrox.svc:8080/$scenario"

  kubectl -n stackrox rollout status ds/collector --timeout=3m
  sleep 20

  kubectl -n stackrox logs deployment/repo2cpe-smoke \
    > "$SMOKE_DIR/requests.jsonl"

  python3 - "$scenario" "$EXPECTED_NODES" "$SMOKE_DIR/requests.jsonl" <<'PY'
import collections, json, pathlib, sys

scenario, expected_nodes, filename = sys.argv[1:]
groups = collections.defaultdict(list)
for line in pathlib.Path(filename).read_text().splitlines():
    if not line.startswith("{"):
        continue
    row = json.loads(line)
    if row["path"] == "/" + scenario:
        groups[row["client"]].append(row)

expected = {
    "recovery": [503, 503, 503, 200],
    "exhausted": [503, 503, 503, 503],
    "permanent": [404],
}[scenario]

assert len(groups) == int(expected_nodes), (scenario, len(groups))
for client, rows in sorted(groups.items()):
    statuses = [row["status"] for row in rows]
    delays = [
        round(b["time"] - a["time"], 2)
        for a, b in zip(rows, rows[1:])
    ]
    assert statuses == expected, (client, statuses)
    print(scenario, client, "statuses:", statuses, "waits:", delays)
print("PASS:", scenario, len(groups), "nodes")
PY

  if [ "$scenario" = recovery ]; then
    kubectl -n stackrox logs deployment/sensor \
      --since-time="$CASE_START" | grep 'Received index report'
  else
    kubectl -n stackrox logs -l app=collector -c compliance \
      --prefix --tail=100 --max-log-requests=20 \
      | grep 'Error creating node index:'
  fi
done

Observed:

PASS: recovery 6 nodes
PASS: exhausted 6 nodes
PASS: permanent 6 nodes

Recovery delivered index reports containing 572 packages (572 deprecated) to Sensor. Both failure scenarios logged rhel: unable to create a mappingFile object on all six nodes.

  1. Restore the default mapping URL and initial scan delay, then remove the fixture:
kubectl -n stackrox set env ds/collector --containers=compliance \
  ROX_NODE_INDEX_MAPPING_URL- \
  ROX_NODE_SCANNING_MAX_INITIAL_WAIT-

kubectl -n stackrox rollout status ds/collector --timeout=3m

kubectl -n stackrox delete \
  deployment/repo2cpe-smoke \
  service/repo2cpe-smoke \
  configmap/repo2cpe-smoke

kubectl -n stackrox get ds collector

Cleanup completed; Collector was restored and 6/6 ready.

Limit: Full vulnerability matching was not verified because Scanner V4’s initial vulnerability-data import was still running.

@openshift-ci

openshift-ci Bot commented Oct 15, 2026

Copy link
Copy Markdown

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

Pin the ClairCore fork's retry implementation and enable three retries
for repo-to-CPE downloads with the existing HTTP client and timeout.

AI-Assisted: Codex, integration change
Signed-off-by: Piotr Rygielski <114479+vikin91@users.noreply.github.com>
@github-actions

github-actions Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

🚀 Build Images Ready

Images are ready for commit 2ef2494. To use with deploy scripts:

export MAIN_IMAGE_TAG=5.1.x-277-g2ef2494dfa

@coderabbitai

coderabbitai Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Central YAML (base), Organization UI (inherited)
  • Review profile: CHILL
  • Plan: Enterprise
  • Run ID: 7ace44af-16e9-455d-b0de-22f22d465068
📥 Commits

Reviewing files that changed from the base of the PR and between f462ad4 and d0c4e64.

⛔ Files ignored due to path filters: 1
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing: 3
  • compliance/node/index/indexer.go
  • compliance/node/index/indexer_test.go
  • go.mod

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

📝 Summary

Summary by CodeRabbit

  • Bug Fixes
    • Repository scans now retry transient service failures up to three times, improving recovery when the service is temporarily unavailable. Permanent not-found errors are not retried.

The repository scanner now sets MaxRetries to 3. Tests cover retry recovery, retry limits, non-retryable errors, and timeout behavior. The module replacement points Claircore to a specified fork version.

Changes: Repository scanner retries

Layer / File(s) Summary
Retry configuration and validation
go.mod, compliance/node/index/indexer.go, compliance/node/index/indexer_test.go
The module replacement selects a Claircore fork. The scanner sets MaxRetries to 3. Tests check recovery after transient errors, the retry limit, not-found responses, and timeout behavior.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: janisz, jvdm

Merge Risk: 🔵 Low · up to d0c4e

This change enables retries for repository mapping downloads by pointing ClairCore at a personal fork. The behavior looks supported, but maintainers should be aware of the fork dependency and plan to move it upstream or into an org-owned repository.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (1 skipped: 1 … Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Title check Passed The title clearly identifies the node-indexer change and its focus on exercising ClairCore mapping-download retries.
Description check Passed The description includes all required sections and provides detailed manual validation for recovery, retry exhaustion, permanent errors, cancellation behavior, and cleanup. Several template checkboxes…
Full details: Description check

Explanation

The description includes all required sections and provides detailed manual validation for recovery, retry exhaustion, permanent errors, cancellation behavior, and cleanup. Several template checkboxes remain unchecked, including automated testing and documentation items, but the description is otherwise complete and directly related to the changes.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 51.97%. Comparing base (f462ad4) to head (2ef2494).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #23436      +/-   ##
==========================================
- Coverage   52.00%   51.97%   -0.04%     
==========================================
  Files        2912     2912              
  Lines      183385   183386       +1     
==========================================
- Hits        95372    95314      -58     
- Misses      79684    79725      +41     
- Partials     8329     8347      +18     
Flag Coverage Δ
go-unit-tests 51.97% <100.00%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant