Skip to content

chore: use nginx module from charmlibs instead of coordinated-workers - #351

Draft
lucabello with Copilot wants to merge 7 commits into
mainfrom
copilot/remove-old-nginx-library
Draft

lucabello with Copilot wants to merge 7 commits into
mainfrom
copilot/remove-old-nginx-library

Conversation

Copilot AI commented Apr 30, 2026

Copy link
Copy Markdown

Issue

The nginx module was extracted from coordinated-workers into the standalone charmlibs.nginx_k8s PyPI package. This charm was still importing nginx utilities directly from coordinated_workers.nginx, which no longer exists in v4.0.0.

Solution

Bump coordinated-workers to >=4.0.0 in both coordinator and worker, and migrate all nginx-related imports to charmlibs.nginx_k8s.

Import migration:

  • coordinator/src/charm.py: from charmlibs.nginx_k8s import NginxConfig, TLSConfigManager — cert path constants now derived as TLSConfigManager.{CA_CERT,CERT,KEY}_PATH
  • coordinator/src/nginx_config.py: from charmlibs.nginx_k8s import NginxLocationConfig, NginxUpstreamignore_worker_role renamed to ignore_address_lookup_key

API removals in v4.0.0:

  • ClusterRolesConfig no longer accepts recommended_deployment — removed from pyroscope_config.py along with the now-dead RECOMMENDED_DEPLOYMENT constant
  • Coordinator._default_degraded_message removed — cleaned up from PyroscopeCoordinator

Test updates:

  • Add Exec(["nginx", "-s", "reload"]) to nginx container fixtures (coordinator unit + interface)
  • Remove test_recommended from test_coherence.py (Coordinator.is_recommended no longer exists)
  • Update test_happy_status_message — expected message changes from "[degraded] UI ready at ..." to "UI ready at ..." (degraded status concept removed upstream)
  • Patch Worker._is_readiness_check_failing in worker endpoint_ready fixture

Context

coordinated-workers 4.0.0 extracted the nginx configuration layer into charmlibs-nginx-k8s (a standalone PyPI package). The recommended_deployment concept and associated is_recommended property were also dropped from Coordinator/ClusterRolesConfig in this release. This follows the same migration done for Tempo in tempo-operators#315.

Testing Instructions

cd coordinator && tox -e unit
cd worker && tox -e unit

Upgrade Notes

No user-facing changes. The nginx configuration behaviour is identical; only the library source changed.

Copilot AI linked an issue Apr 30, 2026 that may be closed by this pull request
Copilot AI changed the title [WIP] Refactor to use nginx module from charmlibs chore: use nginx module from charmlibs instead of coordinated-workers Apr 30, 2026
Copilot AI requested a review from lucabello April 30, 2026 09:31
@lucabello

Copy link
Copy Markdown
Contributor

@copilot Can you address conflicts?

…inx-library

# Conflicts:
#	coordinator/src/charm.py
#	coordinator/src/nginx_config.py
#	coordinator/uv.lock
#	worker/uv.lock

Copilot AI commented Jun 30, 2026

Copy link
Copy Markdown
Author

@copilot Can you address conflicts?

Conflicts resolved in 19d1c20. The merge brought in latest main changes:

  • coordinator/src/charm.py: removed the module-level CA_CERT_PATH/CERT_PATH/KEY_PATH constants (main already uses TLSConfigManager.* attributes directly throughout the file)
  • coordinator/src/nginx_config.py: kept ignore_address_lookup_key=True on the worker upstream
  • coordinator/uv.lock, worker/uv.lock: regenerated via uv lock

All unit tests pass (63 coordinator, 23 worker).

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.

use nginx module from charmlibs

3 participants