Skip to content
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

[nbxplorer] New #7

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

[nbxplorer] New #7

wants to merge 6 commits into from

Conversation

bboerst
Copy link
Owner

@bboerst bboerst commented Sep 10, 2023

Adds nbxplorer. A dependency for btcpayserver.

Based on: https://github.com/dgarage/NBXplorer

@bboerst bboerst force-pushed the adding-nbxplorer branch 3 times, most recently from f6de561 to 79588aa Compare September 11, 2023 21:21
# - name: http
# containerPort: 80
# protocol: TCP
# livenessProbe:
Copy link
Owner Author

Choose a reason for hiding this comment

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

Implement liveness/readiness

@@ -0,0 +1,259 @@
</configuration>root@nbxplorer-78b8df9bfc-xnmq7:/app# ./NBXplorer --help
NBXplorer
Copy link
Owner Author

@bboerst bboerst Sep 12, 2023

Choose a reason for hiding this comment

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

Do we need this .options file?

@@ -0,0 +1,28 @@
apiVersion: v2
Copy link
Owner Author

@bboerst bboerst Sep 12, 2023

Choose a reason for hiding this comment

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

Do some cleanup to this Chart.yaml, mostly just remove comments.

# This is the chart version. This version number should be incremented each time you make changes
# to the chart and its templates, including the app version.
# Versions are expected to follow Semantic Versioning (https://semver.org/)
version: 0.1.5
Copy link
Owner Author

Choose a reason for hiding this comment

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

Probably reset to 1.0.0 before merging

volumeMounts:
- mountPath: /data
name: nbxplorer
# ports:
Copy link
Owner Author

Choose a reason for hiding this comment

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

What's up with this commented out port def?

- "--rmqvirtual=/"
- "--rmqtranex=NewTransaction"
- "--rmqblockex=NetBlock"
- "--postgres=User\ ID={{ .Values.postgresql.auth.username }};Password={{ .Values.postgresql.auth.password }};Host=btcpayserver-postgresql;Port=5432;Database={{ .Values.postgresql.auth.database }}"
Copy link
Owner Author

Choose a reason for hiding this comment

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

postgres config should definitely be pulled out of here so that a user can use a different db. Should probably also put the postgres install behind a flag. Example might be that user wants to use a managed service like Aurora or something.

Copy link
Owner Author

Choose a reason for hiding this comment

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

I think this actually uses the postgres that is stood up with btcpayserver. Might want to flex this dependency a bit.

@@ -0,0 +1,17 @@
{{- if .Values.persistence.enabled }}
Copy link
Owner Author

Choose a reason for hiding this comment

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

Do we need persistence like this? Isn't state stored in pg?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Mounts to mountPath: /data. What is in here that is stateful?

containers:
- name: wget
image: busybox
command: ['wget']
Copy link
Owner Author

Choose a reason for hiding this comment

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

Maybe make this test better. Is there some kind of readiness flag or acceptance criteria that we can check for?

- "--btcrpcauth={{ .Values.config.rpcauth.username }}:{{ .Values.config.rpcauth.password }}"
- "--rmquser={{ .Values.rabbitmq.auth.username }}"
- "--rmqpass={{ .Values.rabbitmq.auth.password }}"
- "--rmqhost=nbxplorer-rabbitmq"
Copy link
Owner Author

Choose a reason for hiding this comment

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

Make this configurable so that an external rmq server could be used here. Maybe put the rabbitmq install behind a flag.

args:
- "--datadir=/data"
- "--bind=0.0.0.0:32838"
- "--network=mainnet"
Copy link
Owner Author

Choose a reason for hiding this comment

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

Should be configurable.

- "--bind=0.0.0.0:32838"
- "--network=mainnet"
- "--chains=btc"
- "--noauth"
Copy link
Owner Author

Choose a reason for hiding this comment

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

What is noauth? Sounds dangerous.

- "--chains=btc"
- "--noauth"
- "--btcrpcurl=http://bitcoind.bitcoin.svc.cluster.local:8332/"
- "--btcnodeendpoint=bitcoind.bitcoin.svc.cluster.local:8333"
Copy link
Owner Author

Choose a reason for hiding this comment

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

btcnodeendpoint and btcrpcurl should be configurable. Also why is one using 8333 and the other is using 8332? Secure vs unsecure...

@bboerst bboerst force-pushed the adding-nbxplorer branch 2 times, most recently from 5097600 to 55bbc82 Compare September 12, 2023 22:55
@bboerst bboerst force-pushed the adding-nbxplorer branch from 55bbc82 to 51ff5ed Compare June 12, 2024 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant