Skip to content

Pre-build ldap image for integration test#92

Merged
lubomir merged 2 commits into
mainfrom
overseer/89
Jul 3, 2026
Merged

Pre-build ldap image for integration test#92
lubomir merged 2 commits into
mainfrom
overseer/89

Conversation

@lubomir

@lubomir lubomir commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

🤖 This was posted automatically by an AI agent.

Summary

Pre-builds the LDAP server used by the integration test as a dedicated container image instead of performing a live pip install ldaptor twisted on every test run.

Changes

New files

  • integration-tests/images/ldap-server/server.py – the existing in-memory LDAP server script (extracted verbatim from the inline ConfigMap in the integration-test pipeline). Listens on port 1389 as an arbitrary UID, compatible with OpenShift's restricted-v2 SCC.

  • integration-tests/images/ldap-server/Containerfile – builds a fedora-minimal:40-based image that installs ldaptor and twisted via pip3 and sets CMD ["python3", "/app/server.py"].

  • .tekton/ldap-server-pull-request.yaml – Konflux build PipelineRun for pull requests. Triggers only when files under integration-tests/images/ldap-server/ change (pathChanged() CEL filter). Uses service account build-pipeline-ldap-server and sets image-expires-after: 5d.

  • .tekton/ldap-server-push.yaml – Konflux build PipelineRun for pushes to main. Same path filter. Applies the main tag via the apply-tags task.

Modified file

  • .tekton/integration-test-eaas.yaml:
    • parse-snapshot: adds a ldap-server-image-url result that extracts the ldap-server component's containerImage from the Snapshot JSON.
    • deploy-openldap: replaces the inline ConfigMap creation, pip install, volume mount, and volumes with a single Deployment that references the pre-built image passed as the ldap-server-image parameter.

@codecov-commenter

codecov-commenter commented Jul 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.69%. Comparing base (37bad18) to head (00048f5).

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #92   +/-   ##
=======================================
  Coverage   83.69%   83.69%           
=======================================
  Files          13       13           
  Lines        1325     1325           
=======================================
  Hits         1109     1109           
  Misses        216      216           
Flag Coverage Δ
unit-tests 83.69% <ø> (ø)

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:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@lubomir

lubomir commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

The plan made some incorrect assumptions about how the tests are going to run. Apparently when more than one component is built in a PR, each one gets a separate test pipeline, plus there is one "group" test pipeline.

In each case the ITS is supposed to get a snapshot with all components. For pipelines running on a single component, the other components are supposed to be populated from latest stable build.

In this case, that is not working as there is no stable build for the ldap-server component.

We get:

  • group: both images present from this PR, tests pass
  • just ldap-server: snapshot uses cts from latest main commit and tests work
  • just cts: snapshot doesn't have ldap-server, and the tests fail

The fallback in the ldap-server extraction is not helpful. If there was something to fall back to, it would have been included in the Snapshot in the first place. If the snapshot doesn't contain ldap-server, the test should just be skipped.

This is only an issue for the initial PR. Once there is an ldap-server built from main branch, the snapshot should always have some url available. This is already visible on the ldap-server ITS run: cts is coming from last build on main.

Build a dedicated container image for the in-memory LDAP server used by
the integration test instead of performing a live 'pip install ldaptor twisted'
on every test run.

New files:
- integration-tests/images/ldap-server/server.py: the in-memory LDAP server
  script, listening on port 1389 as an arbitrary UID (OpenShift restricted-v2
  SCC compatible).
- integration-tests/images/ldap-server/Containerfile: builds a
  fedora-minimal:44-based image that installs ldaptor and twisted via pip3.
- .tekton/ldap-server-pull-request.yaml: Konflux build PipelineRun for pull
  requests; triggers only when files under integration-tests/images/ldap-server/
  change (pathChanged() CEL filter). Sets image-expires-after to 5d.
- .tekton/ldap-server-push.yaml: Konflux build PipelineRun for pushes to main.
  Same path filter.

Modified:
- .tekton/integration-test-eaas.yaml: parse-snapshot now emits a
  ldap-server-present result; when ldap-server is absent from the Snapshot
  (no stable build yet, and not triggered for this PR), every downstream task
  is skipped via a 'when' condition and the finally task emits 'skipped' instead
  of failing.

Generated-By: OpenCode (google-vertex-anthropic/claude-sonnet-4-6@default)
@lubomir

lubomir commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

@guillermodotn , this implements your suggestion of moving the ldap test server into a pre-built image.

It slightly complicates the compatibility story though. If something about the test data changes, there will be multiple pipelines with the latest test code, but multiple combinations of images (new ldap-server+old cts; old ldap-server+new cts; both new images). Either we would need to care to make all of these work, or accept having failing tests.

The check to skip the pipeline if ldap-server is missing is needed only for the initial PR. Once (if) this is merged, there would always be some ldap-server image available.


RUN microdnf install -y python3 python3-pip && \
microdnf clean all && \
pip3 install --no-cache-dir ldaptor twisted

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.

maybe we should pin the versions in here or add a requirements.txt under integration-tests/images/ldap-server

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, this is a good point. The build could be switched to hermetic mode too when the file is added. But I'd prefer to do that in a separate follow up PR to not bloat this even more.

Comment thread integration-tests/images/ldap-server/Containerfile
@guillermodotn

Copy link
Copy Markdown
Contributor

@guillermodotn , this implements your suggestion of moving the ldap test server into a pre-built image.

It slightly complicates the compatibility story though. If something about the test data changes, there will be multiple pipelines with the latest test code, but multiple combinations of images (new ldap-server+old cts; old ldap-server+new cts; both new images). Either we would need to care to make all of these work, or accept having failing tests.

The check to skip the pipeline if ldap-server is missing is needed only for the initial PR. Once (if) this is merged, there would always be some ldap-server image available.

I guess we can extract the LDIF data to its own file and mount it at runtime via a ConfigMap, so the image only contains the server dependencies and never goes stale.

What do you think?

@lubomir

lubomir commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

I guess we can extract the LDIF data to its own file and mount it at runtime via a ConfigMap, so the image only contains the server dependencies and never goes stale.

That's a great idea. It's going to need a bit of refactoring though. The deploy-ldap task currently doesn't have access to the files from the git repo. Those files are only needed for the run-tests task, which does a git clone in the test runner ephemeral container. I guess the solution would be to have a separate task to clone the git repo into a workspace. Then all deploy tasks would have access to it and could read config files. The clone in run-tests could be replaced with kubectl cp so that we only clone once.

With such change, the deploy tasks could read manifests from files, and no longer embed them. The current yaml in bash in yaml is not the most readable thing...

I think that refactor should be a separate PR. We can block this one until that's done and then rebase, or merge this and then refactor. I'm happy to do either.

@lubomir lubomir merged commit e3ec3bc into main Jul 3, 2026
9 checks passed
@lubomir lubomir deleted the overseer/89 branch July 3, 2026 13:26
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.

3 participants