budimanjojo / talhelper

A tool to help creating Talos kubernetes cluster
https://budimanjojo.github.io/talhelper
BSD 3-Clause "New" or "Revised" License
287 stars 17 forks source link

New user onboarding: initialise content #492

Closed PrivatePuffin closed 3 months ago

PrivatePuffin commented 4 months ago

When a new user runs talhelper in a folder that doesn't contain any config yet, maybe it should propose to generate it from examples to "onboard" the user? Instead of just stating the fact its missing.

budimanjojo commented 4 months ago

Thank you! I think I should create a talhelper init subcommand and prompt the user to a yes and no question when there's no config file found. Do you think my solution suits your expectation?

PrivatePuffin commented 4 months ago

Thank you! I think I should create a talhelper init subcommand and prompt the user to a yes and no question when there's no config file found. Do you think my solution suits your expectation?

Yeah this is awesome! :)

That should set new users op with a somewhat useable/reference-able file structure to work from... 👍

budimanjojo commented 4 months ago

I'll try to do this when I have the time, thank you!

PrivatePuffin commented 4 months ago

NP! Looking forward! ^^

mircea-pavel-anton commented 4 months ago

@Ornias1993

Personally, I don't really see much value in this, and I'd much rather maybe keep the example config in the docs updated instead

I feel like an init function that creates an empty talhelper config, an empty talsecret and an empty talenv is not really all that useful. The docs should walk you through it rather than the tool, and then taking it further to an interactive experience to also populate those files is a bit too much again

PrivatePuffin commented 4 months ago

@Ornias1993

Personally, I don't really see much value in this, and I'd much rather maybe keep the example config in the docs updated instead

As a devops engineer it obviously doesn't have value to you, I assume you know how to read docs and apply them.

However, the average user... Or can I say "The average idiot", because quite frankly whom these functions are for, often needs a little "nudge" on what goes where.

mircea-pavel-anton commented 4 months ago

@Ornias1993

Personally, I don't really see much value in this, and I'd much rather maybe keep the example config in the docs updated instead

As a devops engineer it obviously doesn't have value to you, I assume you know how to read docs and apply them.

However, the average user... Or can I say "The average idiot", because quite frankly whom these functions are for, often needs a little "nudge" on what goes where.

Firstly, I don't really agree with your choice of words, but that's besides the point

Secondly, while I do see your point, I'm not so sure the "average user" you speak of would be using templating tools to automate the declarative configuration of their immutable, API-drives OS :)))

I think this tool is targeted more towards users with a certain level of experience reading documentation and writing YAML manifests. I wouldn't say that someone running Kubernetes clusters is incapable of reading docs

PrivatePuffin commented 4 months ago

I think this tool is targeted more towards users with a certain level of experience reading documentation and writing YAML manifests

While I agree with that compeletely...

I wouldn't say that someone running Kubernetes clusters is incapable of reading docs

You would be awkwardly surprised on that one ;-)


Personally working on onboarding multiple people onto Talos through Talhelper, through custom tooling. So I personally handled this specific issue outside of Talhelper.

This is issue was just a nice hint from something that I noticed. Its not my personal interest nor do I have any interest in debating it to be frank.

mircea-pavel-anton commented 4 months ago

As you wish. I personally vote for updating the docs. Perhaps the example from the command line banner too?

It's up to @budimanjojo to decide

I can update the docs in the near future, as I need to work on updating my talhelper and redeploying my cluster soon as well. I can provide examples then, if needed.

budimanjojo commented 4 months ago

I have actually already have a talhelper init subcommand locally that will create a file with this content:

 go run . init
example config file "./talconfig.yaml" generated
budiman@budimanjojo-main in ~/G/talhelper on  master       
 cat talconfig.yaml
---
# This is an example config file for `talhelper` so not every available keys are listed here.
# See the documentation https://budimanjojo.github.io/talhelper/latest/reference/configuration/ for all the available keys.
# You can run `talhelper genconfig` after you're done editing this file.
# The generated manifest files will be in `./clusterconfig` directory
# Make sure to run `talhelper gensecret > talsecret.sops.yaml` and don't forget to encrypt it using `sops`.
# This will make sure the secrets to be persisted everytime you run `talhelper genconfig` command.
# You can also have a `talenv.yaml` file that stores non secret variables and use it as value with `${varName}`.

clusterName: example-cluster # the cluster's name (required)
endpoint: https://192.168.200.10:6443 # controlplane endpoint, usually loadbalancer or VIP (required)
talosVersion: v1.7.4 # defaults to the latest Talos version known by this Talhelper version
kubernetesVersion: v1.30.0 # defaults to the version shipped with `talosVersion`
nodes: # list of nodes configurations (required)
  - hostname: controlplane # hostname of the node (required)
    ipAddress: 192.168.200.11 # IP address where the node can be reached (required)
    installDisk: /dev/sda # disk used for installation (required if `installDiskSelector` is not set)
    controlPlane: true # whether the node is a controlplane
    patches: # patches to be applied to the node
      # RFC6902 JSON patch
      - |-
        - op: add
          path: /machine/kubelet/extraArgs
          value:
            rotate-server-certificate: "true"
      # strategic merge patch
      - |-
        machine:
          env:
            MYENV: value
      # file containing RFC6902 JSON or strategic merge patch with `@` prefixed
      - "@./a-patch.yaml"
    schematic: # Talos image customization for the installer image
      customization:
        extraKernelArgs:
          - net.ifnames=0
        systemExtensions:
          officialExtensions:
            - siderolabs/intel-ucode
  - hostname: worker
    ipAddress: 192.168.200.12
    installDiskSelector:
      size: 128GB
      model: WDC*
      name: /sys/block/sda/device/name
      busPath: /pci0000:00/0000:00:17.0/ata1/host0/target0:0:0/0:0:0:0
    controlPlane: false
# everything defined here will be applied to all controlplane nodes
controlPlane:
  # the same as `node[].patches` above
  patches: []
# everything defined here will be applied to all worker nodes
worker:
  # the same as `node[].patches` above
  patches: []

I haven't push this into a PR is because I don't know if this is actually enough or too much or too little. And I also don't think prompting a user an interactive "Y/N" question when doing genconfig subcommand is a good idea because hanging in the terminal waiting for user input will break scripts. So I think error out and suggesting this talhelper init or read the docs to the user is more reasonable.

But still, having an example config that suggests the user to do things like gensecret etc in comments feels weird to me but if that's how new users want to onboard, which I'm not really sure, then I'm fine too, that's why I'm trying this out locally.

Also, @Ornias1993 please don't attack people's profession and nobody is calling average users "idiots". This kind of language is very rude and unneeded.

I think @mirceanton is right, having better docs given to the user while interacting with the program is a better approach. Something like:

-> talhelper genconfig
You don't have a talconfig.yaml, please read the docs here: <link> or run `talhelper init` to create an example file for you

Not sure which one is a better onboarding experience, I'm open to more suggestions if anyone have another better views so please share.

PrivatePuffin commented 4 months ago

As you wish. I personally vote for updating the docs. Perhaps the example from the command line banner too?

It's up to @budimanjojo to decide

I can update the docs in the near future, as I need to work on updating my talhelper and redeploying my cluster soon as well. I can provide examples then, if needed.

I agree that the docs could use a more noob-friendly quick-start regardless though, forgot to mention!

PrivatePuffin commented 4 months ago

@budimanjojo I didnt attack his profession AT ALL. I stated that as a devops engineer i assume he us competent. If anything thats a compliment.

And yes, the average non-professional user is, sadly enough, pretty close to an idiot. Thats a statement of fact, ask anyone dealing with those as support-staffer.

Thats a statement of professional al fact, not a nice one. But a true one nonetheless.

But ill refraign from stating the obvious next time.

budimanjojo commented 4 months ago

I feel like it's just rude and unnecessary to call people that want such feature idiots, and ironically you're requesting this feature for the people you want to onboard Talos into that I assume are your potential customers.

Let's get back into the topic and see if the solution I provided above is good enough.

PrivatePuffin commented 3 months ago

I assume are your potential customers.

Just to be clear:

I dont plan to sell any software or provide paid onboarding services.

I personally dont need this, or even personally want this.

As for my community project well do onboarding... well... differently...

I made the feature request because I feel it would be a great addition for "the average idiot".

So to respond on your question: im not the target audience to answer, because I wont personally use it.

mircea-pavel-anton commented 3 months ago

Let's just keep the conversation on topic and agree on a solution.

I personally vote for errors to include a link to the appropriate documentation page alongside an error message. That seems like the least effort for now and we can improve on that later on if the need arises.

I noticed this same approach in the GitHub actions runner scale set I believe where (some?) errors include a link to their docs and I thought it was quite useful.

Thoughts? @budimanjojo

budimanjojo commented 3 months ago

@mirceanton I agree. I'll create a PR maybe tomorrow and please provide feedback on the PR when it's pushed. Thank you!

budimanjojo commented 3 months ago

I'll just merge the PR (that will close this issue) since I want to make a new release due to new upstream alpha release.