Packet Quickstart: add machine types to the example config

[margamanterola]

Currently it's not possible to deploy the latest lokomotive release on Packet without specifying the machine type. Figuring out the right parameters in the config takes a while. Make this easier by adding them to the example config.

[johananl]

Thanks for the PR @marga-kinvolk.

c3.small.x86 is the default machine type in current master:

lokomotive/assets/lokomotive-kubernetes/packet/flatcar-linux/kubernetes/variables.tf

Lines 43 to 47 in 1afc56c

	variable "controller_type" {
	type = string
	default = "c3.small.x86"
	description = "Packet instance type for controllers"
	}

On v0.2.1 the default is baremetal_0.

My guess is your Packet project doesn't allow baremetal_0 machines. The next release should use c3.small.x86 as the default, and since we're trying to keep the number of knobs the user needs to worry about small in the quickstarts, I'd rather not explicitly specify c3.small.x86.

[margamanterola]

My project (the Flatcar sponsored project) doesn't allow neither baremetal_0 nor c3.small.x86, so even with the new release it wouldn't have worked. And then there's also the fact that Packet locations don't always have availability of all hardware, leading to failure to provision even when the project allows the machine type.

I understand wanting to keep the documentation simple, but I was following the "quick start" guide and had to instead read a rather long and confusing reference page to understand how to change the machine types. Given my experience with Packet so far (for deploying Flatcar infra, outside of Lokomotive) I think that choosing the machine type is something that belongs in the quick start and not in the "For more things read this long and unfriendly page".

Why I say the "Packet configuration reference" is not particularly friendly:

• I was looking for the machine type config and I had to process a huge config with tons of variables and parameters and no explanation (the explanation comes later, I'm not sure why the config is at the top, but it's not what I expected from a "reference").
• There's a variable called workers_type at the top, but the parameter is actually called node_type which needs to be part of the worker_pool. The controller_type is listed with it's type, but the node_type is listed with a variable so it's less evident that this is the machine type.
• Why have all that stuff with the variables at the top? As a newbie to Lokomotive I was pretty puzzled by the variables and kept wondering whether I needed to set a variable or not to change the value I wanted.
• There's an example of how to change the node_type but it doesn't actually work because it's not inside the worker_pool. And said example doesn't talk about controller_type at all.
• Yes, by the time I reached the reference, it was pretty clear what parameters I needed to change, but I had to go through all the other things first... I think just the reference instead of the huge config would actually be more helpful.

Apart from this, the current example config in the quick start includes several parameters that aren't explained at all, so I'm not sure adding a couple of parameters that are self-explanatory means the user would have to "worry" about them.

I asked on Slack about this and my understanding was that adding it to the example config would be helpful. If you think it's definitely not ok, would you agree to add a troubleshooting section about not being able to provision the default machine type and how to change it?

[rata]

I think it's quite important to add defaults for this in the quickstart.

Currently in lokomotive, as discussed in the 0.3.0 release PR, if the controller machine type is changed, your cluster will be completely broken and etcd data totally gone. For workers, you will experience downtime.

Therefore, I would not ever advice anyone to create a cluster without defaults, as the quickstart is doing.

We can (and should!) create an issue for this, but as long as the currrent behavior is present, no example should rely on defaults for controller nodes.

[rata]

LGTM.

Added one question that I is probably trivial to resolve without changes, so marking as LGTM.

Let me know if changes are needed and I can look again :)

[rata]

@johananl @surajssd @invidian friendly ping?

[johananl]

@marga-kinvolk true, the config reference could definitely use an overhaul. It's tracked here: #368

AFAICT the main use for a default value for the machine types is when deploying test clusters, likely using the quickstart. That's arguably the only case where the user might say "just give me cluster, I don't care about the hardware". So, following the above maybe we should just make the machine type knobs required (and add values to the config sample in the quickstart like you've done here).

[rata]

@johananl you are completely ignoring my comment in your reasoning. I think what you said is just not true if you take into account my comment.

I also disagree with what you are saying. Someone creating a cluster will pay for the resources, so I don't think it is "I don't care, I trust your defaults, give me some hardware". Furthermore, I would not assume that people deploting with a quickstart will throw away their clusters.

[johananl]

@johananl you are completely ignoring my comment in your reasoning. I think what you said is just not true if you take into account my comment.

I also disagree with what you are saying. Someone creating a cluster will pay for the resources, so I don't think it is "I don't care, I trust your defaults, give me some hardware". Furthermore, I would not assume that people deploting with a quickstart will throw away their clusters.

@rata looks like a miscommunication here 🙂 I've read your comment and took it into account.

I'm OK with adding the machine types to the docs, and suggested that in addition we consider making the knobs required since I can't think of cases where there user wouldn't want to determine the machine type (i.e. the user will likely always want to decide what hardware to use and pay for).

Seems to me we are aligned about adding the values to the quickstart. I'll leave it up to you to decide if you want to also change these knobs to required (that was a just suggestion).

[rata]

Sorry, as we talked on slack, I completely misread your last sentence.

Cool, IMHO, we should:

• Merge this
• Create an issue to handle this on lokomotive (i.e. not let users break their clusters) in some way (documentation or actual code). If we decide to go with docs, we might want to remove the default value as @johananl mentioned.
• Create an issue to do the same for quickstarts on other platforms

Am I missing something?

[margamanterola]

Hey, can this be merged then? Should I merge it or will one of the Lokomotive folks do it?

[johananl]

I'll merge this and open follow up issues.

[johananl]

The existing quickstart guides don't contain config snippets (rather, they contain a link to a sample config file).

We already have issues for refactoring the remaining quickstarts following the Packet one:

• Refactor AWS quickstart #613
• Refactor bare metal quickstart #614
• Refactor AKS quickstart #615

The description suggests to use the Packet quickstart as a reference so I think we should be good.

[johananl]

Create an issue to handle this on lokomotive (i.e. not let users break their clusters) in some way (documentation or actual code). If we decide to go with docs, we might want to remove the default value as @johananl mentioned.

@rata I'm not sure I understand what you mean here. If you think we need a new issue, could you please open one?

[rata]

Sorry it wasn't clear. I created this issue for that: #803

Thanks!