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

refactor charts #169

Merged
merged 1 commit into from Mar 25, 2021
Merged

refactor charts #169

merged 1 commit into from Mar 25, 2021

Conversation

elric1
Copy link
Collaborator

@elric1 elric1 commented Mar 24, 2021

No description provided.

@nicolasochem
Copy link
Collaborator

Chart refactoring looks OK. It's worth making a dedicated PR for it. I haven't tried it since I have reservations on the second commit.

Just one question, a lot of the newly added templates are wrapped into if statements ("if wait for bootstrap", "if download snapshot"). Did you consider leaving the if statements in the final yaml files for readability?

The second commit breaks the "known network" use case: if you want to launch an edo2net or mainnet node, config-generator will run tezos-node config init in order to extract data that is needed but trapped in the binary, such as the chain name and bootstrap peers. So it can't be merged as is. One solution is to move this step into its own init container that will be built on top of tezos. Private chains will never need this container so you can skip building it.

@elric1
Copy link
Collaborator Author

elric1 commented Mar 25, 2021

I've removed the second commit. I only put it in this PR because it will conflict if I based it off head. Let's consider this one on its own. I'll keep on working on the image one separately.

I did consider putting the conditionals in the final YAML files and, in fact, that is how I originally did it. I found it to be less readable and also more repetitive because the same if statements would be repeated in both baker.yaml and node.yaml.

@nicolasochem
Copy link
Collaborator

I tried a zerotier invite and:

Events:
  Type     Reason            Age                 From                    Message
  ----     ------            ----                ----                    -------
  Normal   SuccessfulCreate  65s                 statefulset-controller  create Claim var-volume-tezos-node-0 Pod tezos-node-0 in StatefulSet tezos-node success
  Warning  FailedCreate      24s (x14 over 65s)  statefulset-controller  create Pod tezos-node-0 in StatefulSet tezos-node failed error: Pod "tezos-node-0" is invalid: spec.initContainers[2].name: Duplicate value: "get-zerotier-ip"

@nicolasochem
Copy link
Collaborator

The above happened on the invite chart and this is the event log for tezos-node statefulset.

if you run mkchain with --zerotier-network and --zerotier-token, it will give you a file whatever_invite_values.yaml

if you pass this file to helm template, I expect that you will see a duplicate init container in the node staetefulset

@elric1
Copy link
Collaborator Author

elric1 commented Mar 25, 2021

Yes, thanks. I did that and found the offending line. I included the zerotier template twice by default.

I then tested again with rpc_auth set to true---and also with the snapshot-downloader. These appear to generate the correct charts, now.

@nicolasochem nicolasochem self-requested a review March 25, 2021 21:22
Copy link
Collaborator

@nicolasochem nicolasochem left a comment

Choose a reason for hiding this comment

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

Thank you. I created a zerotier chain with mkchain and was able to spin it up and join it from another workspace, and I have no reason to believe anything else is broken.

@elric1 elric1 merged commit ddfd391 into oxheadalpha:master Mar 25, 2021
name: var-volume
{{- end }}

{{- define "tezos.container.baker" }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I know I'm very late to the party here but just wanted to throw out, that being only baker is is running baker/endorser, why not have this explicit there. It can still include "tezos.localvars.pod_envvars".

Also, was there any more discussion regarding having the if checks in the templates themselves?

@elric1 elric1 deleted the refactor-charts branch April 28, 2021 14:19
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.

None yet

3 participants