-
Notifications
You must be signed in to change notification settings - Fork 52
Circuit breaker plugin #688
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
73edcab
63d41bb
83f9f41
caeb5ad
9d42a45
bccc111
9bcadb6
370a830
01c04f1
a1c8645
51e2f61
00ce51c
8cecc14
067d98e
f2f23d7
a2f25aa
aef4f9c
0418553
a1bbd23
91da2a1
6745906
61da106
8572824
58257a9
acf25a1
8e7b135
8c541b1
b775b6b
bb663f8
746b9f7
914ba77
f0b0b9b
074192e
dc56158
03954ca
b634e1e
64c1f1f
dc82fa4
c265bb1
9035bd6
46bb5b6
c4289d1
0e440f5
b4d66e7
08903ec
c044f17
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -58,16 +58,37 @@ spec: | |
- mountPath: /root/.lnd/tls.cert | ||
name: config | ||
subPath: tls.cert | ||
- name: shared-volume | ||
mountPath: /root/.lnd/ | ||
{{- with .Values.extraContainers }} | ||
{{- toYaml . | nindent 4 }} | ||
{{- end }} | ||
{{- if .Values.circuitbreaker.enabled }} | ||
- name: circuitbreaker | ||
Camillarhi marked this conversation as resolved.
Show resolved
Hide resolved
|
||
image: {{ .Values.circuitbreaker.image | quote }} | ||
imagePullPolicy: IfNotPresent | ||
args: | ||
- "--network={{ .Values.global.chain }}" | ||
- "--rpcserver=localhost:{{ .Values.RPCPort }}" | ||
- "--tlscertpath=/tls.cert" | ||
- "--macaroonpath=/root/.lnd/data/chain/bitcoin/{{ .Values.global.chain }}/admin.macaroon" | ||
- "--httplisten=0.0.0.0:{{ .Values.circuitbreaker.httpPort }}" | ||
volumeMounts: | ||
- name: shared-volume | ||
mountPath: /root/.lnd/ | ||
- name: config | ||
mountPath: /tls.cert | ||
subPath: tls.cert | ||
{{- end }} | ||
Comment on lines
+66
to
+82
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Was there an issue including circuitbreaker as extraContainers container? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I was not able to access the value of the port and network variables as the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm the whole point of those There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I understand, but I could not access a dynamic network or port while using it. The current approach also gives the option of enabling circuitbreaker if needed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @pinheadmz I assume it is acceptable to define network(chain) in network.yaml or node-defaults.yaml based on user desired scenario. if so then extraContainers should be possible... |
||
volumes: | ||
{{- with .Values.volumes }} | ||
{{- toYaml . | nindent 4 }} | ||
{{- end }} | ||
- configMap: | ||
name: {{ include "lnd.fullname" . }} | ||
name: config | ||
- name: shared-volume | ||
emptyDir: {} | ||
{{- with .Values.nodeSelector }} | ||
nodeSelector: | ||
{{- toYaml . | nindent 4 }} | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -2,9 +2,13 @@ | |||||||||||||
|
||||||||||||||
import json | ||||||||||||||
import os | ||||||||||||||
import random | ||||||||||||||
import subprocess | ||||||||||||||
import time | ||||||||||||||
from pathlib import Path | ||||||||||||||
from time import sleep | ||||||||||||||
|
||||||||||||||
import requests | ||||||||||||||
from test_base import TestBase | ||||||||||||||
|
||||||||||||||
from warnet.process import stream_command | ||||||||||||||
|
@@ -24,11 +28,18 @@ def __init__(self): | |||||||||||||
"tank-0005-ln", | ||||||||||||||
] | ||||||||||||||
|
||||||||||||||
self.cb_port = 9235 | ||||||||||||||
self.cb_node = "tank-0003-ln" | ||||||||||||||
self.port_forward = None | ||||||||||||||
|
||||||||||||||
def run_test(self): | ||||||||||||||
try: | ||||||||||||||
# Wait for all nodes to wake up. ln_init will start automatically | ||||||||||||||
self.setup_network() | ||||||||||||||
|
||||||||||||||
# Test circuit breaker API | ||||||||||||||
self.test_circuit_breaker_api() | ||||||||||||||
|
||||||||||||||
# Send a payment across channels opened automatically by ln_init | ||||||||||||||
self.pay_invoice(sender="tank-0005-ln", recipient="tank-0003-ln") | ||||||||||||||
|
||||||||||||||
|
@@ -39,6 +50,7 @@ def run_test(self): | |||||||||||||
self.pay_invoice(sender="tank-0000-ln", recipient="tank-0002-ln") | ||||||||||||||
|
||||||||||||||
finally: | ||||||||||||||
self.cleanup_kubectl_created_services() | ||||||||||||||
self.cleanup() | ||||||||||||||
|
||||||||||||||
def setup_network(self): | ||||||||||||||
|
@@ -120,6 +132,113 @@ def scenario_open_channels(self): | |||||||||||||
self.log.info(f"Running scenario from: {scenario_file}") | ||||||||||||||
self.warnet(f"run {scenario_file} --source_dir={self.scen_dir} --debug") | ||||||||||||||
|
||||||||||||||
def test_circuit_breaker_api(self): | ||||||||||||||
self.log.info("Testing Circuit Breaker API") | ||||||||||||||
|
||||||||||||||
# Set up port forwarding to the circuit breaker | ||||||||||||||
cb_url = self.setup_api_access(self.cb_node) | ||||||||||||||
|
||||||||||||||
self.log.info(f"Testing Circuit Breaker API at {cb_url}") | ||||||||||||||
|
||||||||||||||
# Test /info endpoint | ||||||||||||||
info = self.cb_api_request(cb_url, "get", "/info") | ||||||||||||||
assert "version" in info, "Circuit breaker info missing version" | ||||||||||||||
|
||||||||||||||
# Test /limits endpoint | ||||||||||||||
limits = self.cb_api_request(cb_url, "get", "/limits") | ||||||||||||||
assert isinstance(limits, dict), "Limits should be a dictionary" | ||||||||||||||
|
||||||||||||||
self.log.info("✅ Circuit Breaker API tests passed") | ||||||||||||||
|
||||||||||||||
def setup_api_access(self, pod_name): | ||||||||||||||
"""Set up Kubernetes Service access to the Circuit Breaker API""" | ||||||||||||||
# Create a properly labeled service using kubectl expose | ||||||||||||||
service_name = f"{pod_name}-svc" | ||||||||||||||
|
||||||||||||||
self.log.info(f"Creating service {service_name} for pod {pod_name}") | ||||||||||||||
try: | ||||||||||||||
subprocess.run( | ||||||||||||||
[ | ||||||||||||||
"kubectl", | ||||||||||||||
"expose", | ||||||||||||||
"pod", | ||||||||||||||
pod_name, | ||||||||||||||
"--name", | ||||||||||||||
service_name, | ||||||||||||||
"--port", | ||||||||||||||
str(self.cb_port), | ||||||||||||||
"--target-port", | ||||||||||||||
str(self.cb_port), | ||||||||||||||
], | ||||||||||||||
check=True, | ||||||||||||||
) | ||||||||||||||
except subprocess.CalledProcessError as e: | ||||||||||||||
self.log.error(f"Failed to create service: {e.stderr}") | ||||||||||||||
raise | ||||||||||||||
Comment on lines
+159
to
+177
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks. I will do this instead There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is overkill. See the legacy test, you can just execute an API request form inside the cb container. (this exact code wont work any more but maybe gives a good hint) Lines 34 to 39 in 01195c2
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I looked into the legacy test but was unable to replicate it or access There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you could add a util funciton to k8s.py to execute an arbitrary command inside a container (surprised we dont have that alreaady actually....) and then execute the command There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh right or do something like this in the test:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks. I will try this |
||||||||||||||
|
||||||||||||||
time.sleep(51) # Wait for the service to be created | ||||||||||||||
|
||||||||||||||
service_url = f"http://{service_name}:{self.cb_port}/api" | ||||||||||||||
self.service_to_cleanup = service_name | ||||||||||||||
self.log.info(f"Service URL: {service_url}") | ||||||||||||||
|
||||||||||||||
self.log.info(f"Successfully created service at {service_url}") | ||||||||||||||
return service_url | ||||||||||||||
|
||||||||||||||
def cb_api_request(self, base_url, method, endpoint, data=None): | ||||||||||||||
"""Universal API request handler with proper path handling""" | ||||||||||||||
try: | ||||||||||||||
# Parse the base URL components | ||||||||||||||
url_parts = base_url.split("://")[1].split("/") | ||||||||||||||
service_name = url_parts[0].split(":")[0] | ||||||||||||||
port = url_parts[0].split(":")[1] if ":" in url_parts[0] else "80" | ||||||||||||||
base_path = "/" + "/".join(url_parts[1:]) if len(url_parts) > 1 else "/" | ||||||||||||||
|
||||||||||||||
# Set up port forwarding | ||||||||||||||
local_port = random.randint(10000, 20000) | ||||||||||||||
pf = subprocess.Popen( | ||||||||||||||
["kubectl", "port-forward", f"svc/{service_name}", f"{local_port}:{port}"], | ||||||||||||||
stdout=subprocess.PIPE, | ||||||||||||||
stderr=subprocess.PIPE, | ||||||||||||||
) | ||||||||||||||
|
||||||||||||||
try: | ||||||||||||||
# Wait for port-forward to establish | ||||||||||||||
time.sleep(2) | ||||||||||||||
|
||||||||||||||
# Construct the full local URL with proper path handling | ||||||||||||||
full_path = base_path.rstrip("/") + "/" + endpoint.lstrip("/") | ||||||||||||||
local_url = f"http://localhost:{local_port}{full_path}" | ||||||||||||||
|
||||||||||||||
self.log.debug(f"Attempting API request to: {local_url}") | ||||||||||||||
|
||||||||||||||
# Make the request | ||||||||||||||
if method.lower() == "get": | ||||||||||||||
response = requests.get(local_url, timeout=30) | ||||||||||||||
else: | ||||||||||||||
response = requests.post(local_url, json=data, timeout=30) | ||||||||||||||
|
||||||||||||||
response.raise_for_status() | ||||||||||||||
return response.json() | ||||||||||||||
|
||||||||||||||
finally: | ||||||||||||||
pf.terminate() | ||||||||||||||
pf.wait() | ||||||||||||||
|
||||||||||||||
except Exception as e: | ||||||||||||||
self.log.error(f"API request to {local_url} failed: {str(e)}") | ||||||||||||||
raise | ||||||||||||||
|
||||||||||||||
def cleanup_kubectl_created_services(self): | ||||||||||||||
"""Clean up any created resources""" | ||||||||||||||
if hasattr(self, "service_to_cleanup") and self.service_to_cleanup: | ||||||||||||||
self.log.info(f"Deleting service {self.service_to_cleanup}") | ||||||||||||||
subprocess.run( | ||||||||||||||
["kubectl", "delete", "svc", self.service_to_cleanup], | ||||||||||||||
stdout=subprocess.DEVNULL, | ||||||||||||||
stderr=subprocess.DEVNULL, | ||||||||||||||
) | ||||||||||||||
|
||||||||||||||
|
||||||||||||||
if __name__ == "__main__": | ||||||||||||||
test = LNBasicTest() | ||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you might not actually need this shared volume at all. Because lnd and circuitbreaker are in the same pod, they already share a filesystem. So you'd just need to configure circuitbreaker to read from the right location. There's also apparently an issue getting the network value into the config.
When I
warnet deploy resources/networks/hello
I see this error in the circuitbreaker container:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Camillarhi did you try removing the shared volume?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was looking into other comments. I will look into this as soon as I can
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pinheadmz , After testing both approaches, I've confirmed that a shared volume is necessary for reliable file access between containers in the same pod. While containers in a pod share the same network namespace, they maintain separate filesystems unless explicitly shared through volumes.