Skip to content

Use inet6-aware memcached_servers for [keystone_authtoken] in nova.conf - #1217

Open
yushoyamaguchi wants to merge 1 commit into
openstack-k8s-operators:mainfrom
yushoyamaguchi:memcachedserver-in-nova_conf
Open

yushoyamaguchi wants to merge 1 commit into
openstack-k8s-operators:mainfrom
yushoyamaguchi:memcachedserver-in-nova_conf

Conversation

@yushoyamaguchi

@yushoyamaguchi yushoyamaguchi commented Sep 23, 2026 •

Copy link
Copy Markdown

fix #1216

[cache]'s non-TLS branch already renders memcache_servers with the inet6: prefix (.MemcachedServersWithInet), required by python-memcached on IPv6 deployments — otherwise it defaults to AF_INET and DNS resolution fails outright. [keystone_authtoken] was still using the plain .MemcachedServers variable, so token cache lookups never connect and every authenticated request falls through to a live Keystone call after ~30s of failed lookups (3 servers × ~2 failed A-record attempts each).

  • templates/nova/nova.conf: switch [keystone_authtoken]'s memcached_servers to .MemcachedServersWithInet, matching [cache].
  • test/functional/nova/novaapi_controller_test.go: update the two assertions that hardcoded the (buggy) prefix-less expected value to call GetMemcachedServerListWithInetString() instead, same as the neighboring [cache] assertions already do.

@openshift-ci

openshift-ci Bot commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: yushoyamaguchi
Once this PR has been reviewed and has the lgtm label, please assign kk7ds for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci

openshift-ci Bot commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

Hi @yushoyamaguchi. Thanks for your PR.

I'm waiting for a openstack-k8s-operators member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@coderabbitai

coderabbitai Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

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: Advanced
  • Run ID: 3e307523-6d49-4428-8abd-5745a8ab3c5b
📥 Commits

Reviewing files that changed from the base of the PR and between 07352f4 and badf6d1.

📒 Files selected for processing (2)
  • templates/nova/nova.conf
  • test/functional/nova/novaapi_controller_test.go

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


📝 Summary

Summary by CodeRabbit

  • Bug Fixes
    • Nova now uses the appropriate Memcached server addresses based on whether TLS is enabled. When Memcached TLS is configured, Nova also enables Memcached TLS independently of whether a client certificate is configured, ensuring the cache connection settings are applied consistently.

Walkthrough

The Nova configuration template selects Memcached server addresses based on the TLS setting. Functional tests compare the rendered server lists with values from the Memcached instance.

Changes

Memcached server configuration

Layer / File(s) Summary
Memcached configuration and assertions
templates/nova/nova.conf, test/functional/nova/novaapi_controller_test.go
The template uses MemcachedServers when TLS is enabled and MemcachedServersWithInet otherwise. It enables Memcached TLS in the TLS branch. Functional tests check the server lists returned by the Memcached instance and the TLS setting.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to badf6

No actionable defect is established in the reviewed change.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main IPv6-aware Memcached configuration change for [keystone_authtoken] in nova.conf.
Description check ✅ Passed The description directly explains the IPv6 Memcached issue, the template change, and the related test updates. It omits the final TLS conditional detail but remains relevant.
Linked Issues check ✅ Passed Issue [#1216] requires an IPv6-capable memcached_servers value in [keystone_authtoken]. The template now uses .MemcachedServersWithInet when MemcachedTLS is false. It uses .MemcachedServers …
Out of Scope Changes check ✅ Passed All changed files support issue [#1216]. The template conditional prevents the IPv6 fix from changing TLS behavior. The test changes verify both configuration branches. No unrelated operator or servic…
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

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

@centosinfra-prod-github-app

Copy link
Copy Markdown

Build succeeded (check pipeline).
https://gateway-cloud-softwarefactory.apps.ocp.cloud.ci.centos.org/zuul/t/rdoproject.org/buildset/e8d1df9b71c04cbe93c7bc839a5c187e

✔️ openstack-meta-content-provider SUCCESS in 3h 47m 15s
✔️ nova-operator-kuttl SUCCESS in 49m 47s
✔️ nova-operator-kuttl-placement SUCCESS in 39m 26s
✔️ nova-operator-tempest-multinode SUCCESS in 2h 26m 59s
✔️ nova-operator-tempest-multinode-ceph SUCCESS in 2h 45m 02s

@lmiccini

lmiccini commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

Issue with unconditional switch to MemcachedServersWithInet

This PR will fix non-TLS IPv6 deployments, but will break TLS-enabled deployments. Here's why:

Background

MemcachedServers vs MemcachedServersWithInet have different ports:

  • MemcachedServers: Uses port 11212 (TLS) or 11211 (non-TLS) - no inet6: prefix
  • MemcachedServersWithInet: Always uses port 11211 with inet/inet6 prefix

See infra-operator/internal/controller/memcached/memcached_controller.go:671-686

How keystonemiddleware works

  • memcache_tls_enabled=false: Uses python-memcached → needs inet6: prefix for IPv6, connects to port 11211
  • memcache_tls_enabled=true: Uses pymemcache (BMemcacheClientPool) → handles IPv6 natively, connects to port specified in server list

The Problem

When TLS is enabled:

  • Current config sets memcache_tls_enabled=true in [keystone_authtoken]
  • keystonemiddleware uses pymemcache which needs port 11212 (TLS listener)
  • This PR changes to MemcachedServersWithInet which hardcodes port 11211 (non-TLS listener)
  • Result: TLS-enabled keystonemiddleware tries to do TLS handshake on non-TLS port → connection fails

The Correct Fix

Should match the pattern in [cache] section:

{{if .MemcachedTLS}}
memcached_servers={{ .MemcachedServers }}  // Port 11212, pymemcache handles IPv6
memcache_tls_enabled = true
{{else}}
memcached_servers={{ .MemcachedServersWithInet }}  // Port 11211 with inet6: prefix
{{end}}

This way:

  • ✅ Fixes non-TLS IPv6 (uses MemcachedServersWithInet with inet6: prefix)
  • ✅ Keeps TLS working (uses MemcachedServers with port 11212)

Historical Context

Commit 2750f8e switched from MemcachedServersWithInet to MemcachedServers but broke non-TLS IPv6. The fix should be conditional, not switching back unconditionally.

@lmiccini
lmiccini self-requested a review October 8, 2026 16:05

@lmiccini lmiccini left a comment

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.

Should match the pattern in [cache] section:

{{if .MemcachedTLS}}
memcached_servers={{ .MemcachedServers }} // Port 11212, pymemcache handles IPv6
memcache_tls_enabled = true
{{else}}
memcached_servers={{ .MemcachedServersWithInet }} // Port 11211 with inet6: prefix
{{end}}

@yushoyamaguchi
yushoyamaguchi force-pushed the memcachedserver-in-nova_conf branch from 07352f4 to 0dfba4f Compare October 8, 2026 16:56
Use MemcachedServersWithInet in [keystone_authtoken] when memcached
TLS is disabled for IPv6 usecase.

Signed-off-by: Yusho Yamaguchi <ysh.824@outlook.jp>
@yushoyamaguchi
yushoyamaguchi force-pushed the memcachedserver-in-nova_conf branch from 0dfba4f to badf6d1 Compare October 8, 2026 16:57
@yushoyamaguchi

Copy link
Copy Markdown
Author

@lmiccini
Thank you for reviewing!
I've fixed.
Is it okay?

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.

[keystone_authtoken] memcached_servers missing inet6: prefix on IPv6-only deployments

2 participants