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

✨Allow diskSetup to include partition layout #11634

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 22 additions & 1 deletion bootstrap/kubeadm/api/v1beta1/kubeadmconfig_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -675,19 +675,40 @@ type DiskSetup struct {
Filesystems []Filesystem `json:"filesystems,omitempty"`
}

// PartitionSpec defines the size and optional type for a partition
type PartitionSpec struct {
// percentage of disk that partition will take (1-100)
// +kubebuilder:validation:Minimum=1
// +kubebuilder:validation:Maximum=100
// +required
Percentage int32 `json:"percentage"`
Copy link
Contributor

Choose a reason for hiding this comment

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

You should include a required marker to show explicitly that this is required


// partitionType is the numerical value of the partition type (optional)
// +optional
PartitionType *int32 `json:"partitionType,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

What are the valid values for this value? And how would I, as an end user, understand those?

Is there a limit on this? Is 0 a valid value? Are negative values valid?

Copy link
Author

Choose a reason for hiding this comment

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

Actually I can use a string here to reflect the correct linux partition types. It was an attempt to purely map the disk layout as described in the cloud init docs.

Copy link
Contributor

Choose a reason for hiding this comment

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

There's a choice to make here. Either you can allow all of the valid values there, in which case this should be a 2 character lowercase hex value. Or, you can allow specific, common types, and do that by creating an Enum here and creating PascalCase aliases to the formats we actually want to support.

IMO, the latter provides a better UX

}

// Partition defines how to create and layout a partition.
type Partition struct {
// device is the name of the device.
Device string `json:"device"`

// layout specifies the device layout.
// If it is true, a single partition will be created for the entire device.
// When layout is false, it means don't partition or ignore existing partitioning.
Layout bool `json:"layout"`

// diskLayout specifies the percentage of disk space and partition types.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this list ordered?

Can you validate that the percentages sum to 100? Are they allowed to sum to less than 100?

// If specified, this will override the Layout field.
// +optional
DiskLayout []PartitionSpec `json:"diskLayout,omitempty"`

// overwrite describes whether to skip checks and create the partition if a partition or filesystem is found on the device.
// Use with caution. Default is 'false'.
// +optional
Overwrite *bool `json:"overwrite,omitempty"`
// tableType specifies the tupe of partition table. The following are supported:

// tableType specifies the type of partition table. The following are supported:
// 'mbr': default and setups a MS-DOS partition table
// 'gpt': setups a GPT partition table
// +optional
Expand Down
48 changes: 48 additions & 0 deletions bootstrap/kubeadm/api/v1beta1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

133 changes: 83 additions & 50 deletions bootstrap/kubeadm/internal/cloudinit/cloudinit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,66 +127,99 @@ func TestNewInitControlPlaneCommands(t *testing.T) {
}

func TestNewInitControlPlaneDiskMounts(t *testing.T) {
g := NewWithT(t)

cpinput := &ControlPlaneInput{
BaseUserData: BaseUserData{
Header: "test",
PreKubeadmCommands: nil,
PostKubeadmCommands: nil,
WriteFiles: nil,
Users: nil,
NTP: nil,
DiskSetup: &bootstrapv1.DiskSetup{
Partitions: []bootstrapv1.Partition{
{
Device: "test-device",
Layout: true,
Overwrite: ptr.To(false),
TableType: ptr.To("gpt"),
tests := []struct {
name string
input *ControlPlaneInput
expected []string
}{
{
name: "simple layout",
input: &ControlPlaneInput{
BaseUserData: BaseUserData{
Header: "test",
DiskSetup: &bootstrapv1.DiskSetup{
Partitions: []bootstrapv1.Partition{
{
Device: "test-device",
Layout: true,
Overwrite: ptr.To(false),
TableType: ptr.To("gpt"),
},
},
Filesystems: []bootstrapv1.Filesystem{
{
Device: "test-device",
Filesystem: "ext4",
Label: "test_disk",
ExtraOpts: []string{"-F", "-E", "lazy_itable_init=1,lazy_journal_init=1"},
},
},
},
Mounts: []bootstrapv1.MountPoints{
{"test_disk", "/var/lib/testdir"},
},
},
Filesystems: []bootstrapv1.Filesystem{
{
Device: "test-device",
Filesystem: "ext4",
Label: "test_disk",
ExtraOpts: []string{"-F", "-E", "lazy_itable_init=1,lazy_journal_init=1"},
Certificates: secret.Certificates{},
ClusterConfiguration: "my-cluster-config",
InitConfiguration: "my-init-config",
},
expected: []string{
`disk_setup:
test-device:
table_type: gpt
layout: true
overwrite: false`,
},
},
{
name: "detailed disk layout",
input: &ControlPlaneInput{
BaseUserData: BaseUserData{
Header: "test",
DiskSetup: &bootstrapv1.DiskSetup{
Partitions: []bootstrapv1.Partition{
{
Device: "test-device",
DiskLayout: bootstrapv1.DiskLayout{
{Percentage: 33},
{Percentage: 33, PartitionType: ptr.To[int32](82)},
{Percentage: 33},
},
Overwrite: ptr.To(true),
TableType: ptr.To("gpt"),
},
},
},
},
Certificates: secret.Certificates{},
ClusterConfiguration: "my-cluster-config",
InitConfiguration: "my-init-config",
},
Mounts: []bootstrapv1.MountPoints{
{"test_disk", "/var/lib/testdir"},
expected: []string{
`disk_setup:
test-device:
table_type: gpt
layout:
- 33
- [33, 82]
- 33
overwrite: true`,
},
},
Certificates: secret.Certificates{},
ClusterConfiguration: "my-cluster-config",
InitConfiguration: "my-init-config",
}

out, err := NewInitControlPlane(cpinput)
g.Expect(err).ToNot(HaveOccurred())
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
g := NewWithT(t)

expectedDiskSetup := `disk_setup:
test-device:
table_type: gpt
layout: true
overwrite: false`
expectedFSSetup := `fs_setup:
- label: test_disk
filesystem: ext4
device: test-device
extra_opts:
- -F
- -E
- lazy_itable_init=1,lazy_journal_init=1`
expectedMounts := `mounts:
- - test_disk
- /var/lib/testdir`

g.Expect(string(out)).To(ContainSubstring(expectedDiskSetup))
g.Expect(string(out)).To(ContainSubstring(expectedFSSetup))
g.Expect(string(out)).To(ContainSubstring(expectedMounts))
out, err := NewInitControlPlane(tt.input)
g.Expect(err).ToNot(HaveOccurred())

for _, expectedString := range tt.expected {
g.Expect(string(out)).To(ContainSubstring(expectedString))
}
})
}
}

func TestNewJoinControlPlaneAdditionalFileEncodings(t *testing.T) {
Expand Down
11 changes: 11 additions & 0 deletions bootstrap/kubeadm/internal/cloudinit/disk_setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,18 @@ disk_setup:{{ range .Partitions }}
{{- if .TableType }}
table_type: {{ .TableType }}
{{- end }}
{{- if .DiskLayout }}
layout:
{{- range .DiskLayout }}
{{- if .PartitionType }}
- [{{ .Percentage }}, {{ .PartitionType }}]
{{- else }}
- {{ .Percentage }}
{{- end }}
{{- end }}
{{- else }}
layout: {{ .Layout }}
{{- end }}
{{- if .Overwrite }}
overwrite: {{ .Overwrite }}
{{- end -}}
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading