Skip to content

Conversation

@willcl-ark
Copy link
Contributor

  • Move scripts from python files into dedicated scripts (callable from both justfile and cli)
  • Refactor warcli cluster commands:
    • write a small config saving cluster_type and any minikube settings with warcli cluster configure
    • warcli cluster deploy now runs the same script as the justfile, deploying on whatever the connected cluster is
  • update docs

@pinheadmz
Copy link
Contributor

The k8s python lib we use might have a create_from_yaml() method which might replace the need for using python to execute kubectl apply in sub-processes. I say "might" because I'm having trouble finding documentation and haven't tried the method myself yet.

I would love to reduce the mix of shell and python as much as possible, ideally all the justfile commands can just be executed by warcli.

@mplsgrant
Copy link
Collaborator

mplsgrant commented Aug 1, 2024

warcli cluster rocks.

minikube is wanting to use qemu seemingly b/c I use rootless docker. I can suggest a patch based on this: https://github.com/bitcoin-dev-project/warnet/pull/420/files#diff-deb9bb56fb122db0b605aa5b63f95a4665c905b18dd670e1fa6c877576a94ff1R22

I am also getting an ErrImageNeverPull error in kubectl after doing a cluster deploy.

@willcl-ark
Copy link
Contributor Author

minikube is wanting to use qemu seemingly b/c I use rootless docker. I can suggest a patch based on this: #420 (files)

Added a commit with this fix, seems to work just the same for me but I hope fixes for you.

I am also getting an ErrImageNeverPull error in kubectl after doing a cluster deploy.

Hmm, I don't know how you might be getting this. I guess it comes from statefulset-dev.yaml (where we spec to Never pull, as built image should be loaded in local cache). Are you able to provide repro steps/or configuration being used?

@bdp-DrahtBot
Copy link
Collaborator

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #421 (make pip install warnet use prod by willcl-ark)
  • #420 (Quickstart updates by mplsgrant)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@willcl-ark willcl-ark marked this pull request as ready for review August 7, 2024 13:35
@m3dwards
Copy link
Collaborator

m3dwards commented Aug 7, 2024

I really like this. I think it could be improved with some slight changes to make it clear what's for prod and what's for local.

How about:

warcli cluster setup-minikube
warcli cluster deploy --dev --hot-reload (without --dev for production)

Without dev flag, just install the production image same tag as the source code
With dev flag, run kubectl config current-context to see if it's minikube or docker-desktop (or something else) to see how to do the file mounts.

@willcl-ark
Copy link
Contributor Author

OK @m3dwards @pinheadmz, I think this should be a fair bit closer to what you were imagining.

I did not add any --hot (reload) flags, as I think this can be done later. It look quite invasive to do, as we need to propagate the flag all the way into the dockerfile env...

CMD ["warnet", "--dev"]

I am sure it can be done, but not sure I have the stomach for it here 😋

@bdp-DrahtBot bdp-DrahtBot mentioned this pull request Aug 8, 2024
2 tasks
@willcl-ark
Copy link
Contributor Author

@mplsgrant I kept the check for $WAR_RPC you removed in 8f49bd0 "remove unused rpc checks".

I am not actually sure why it seemed to work for me without it, but it should be needed to build the image: https://github.com/bitcoin-dev-project/warnet/pull/419/files#diff-444b1723cb5f02d3a7b6b4616e5e72f841e957f1c0973cc87fa4cb5cc06fdfb2R100

@mplsgrant
Copy link
Collaborator

$WAR_RPC is set in cluster.py and sent into the bash script as an env variable.


# Deploy rpc server
if [ -n "${WAR_DEV+x}" ]; then # Dev mode selector
if [ "$(kubectl config current-context)" = "docker-desktop" ]; then
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we build the dev container on this line?

@m3dwards
Copy link
Collaborator

m3dwards commented Aug 9, 2024

Tested both docker desktop and minikube and all appeared to work.

@m3dwards m3dwards merged commit e5c7df2 into bitcoin-dev-project:main Aug 9, 2024
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.

5 participants