fix EC2 placement zone retrieval [sc-180267]#134
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes EC2 placement zone retrieval for Kubernetes pods discovered through Consul by extracting the availability zone from the external-k8s-topology-zone metadata field. The change enables filtering and ordering of K8s pods by their availability zone, similar to EC2 instances.
Key changes:
- Updated
map_k8s_consul_endpointto extract availability zone from Service metadata - Added test data and assertions to validate AZ retrieval from K8s pods
- Version bumped from 2.11.4 to 2.11.5
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| lib/movable_ink/aws/ec2.rb | Modified map_k8s_consul_endpoint to populate placement.availability_zone from external-k8s-topology-zone metadata instead of nil |
| spec/ec2_spec.rb | Added external-k8s-topology-zone metadata to test data and replaced duplicate assertion with AZ validation |
| lib/movable_ink/version.rb | Bumped version to 2.11.5 |
| Gemfile.lock | Updated MovableInkAWS version to 2.11.5 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| ], | ||
| placement: { availability_zone: nil } | ||
| placement: { availability_zone: endpoint.Service.dig('Meta', 'external-k8s-topology-zone') } |
There was a problem hiding this comment.
The node_meta filter on line 132 filters based on Node.Meta.availability_zone, but for kubernetes pods, the availability zone is stored in Service.Meta['external-k8s-topology-zone']. This means when availability_zone parameter is provided, the Consul query will filter out all kubernetes pods (since they don't have the availability_zone in Node.Meta), but the code will still try to map them and set the placement zone from Service.Meta. Consider either: 1) filtering kubernetes endpoints in application code after retrieval, or 2) handling kubernetes and EC2 endpoints separately with different queries.
Current Behavior
service discovery of k8s pods does not work if you specify an AZ.
Why do we need this change?
We want to discover pods by their AZ.
Implementation Details
Dependencies (if any)
📇 [sc-180267]