Skip to content

config/types/etcd: add support for v3.4 and v3.5 options#16

Merged
jepio merged 2 commits into
flatcar-masterfrom
jepio/etcd-34-35
Feb 8, 2022
Merged

config/types/etcd: add support for v3.4 and v3.5 options#16
jepio merged 2 commits into
flatcar-masterfrom
jepio/etcd-34-35

Conversation

@jepio

@jepio jepio commented Feb 1, 2022

Copy link
Copy Markdown
Member

add support for etcd v3.4 and v3.5 options

The structs were generated by parsing etcd --help output and
processing using a go program that I wrote. The program is available
here: https://gist.github.com/jepio/11f447c4f6270e2a23f66d7fa414f3a6.

Related flatcar/Flatcar#398

How to use

make

Testing done

No warnings when deploying newer versions, and supports new options:

$ cat <<EOF | bin/ct
etcd:
  version: 3.5.1
  experimental_enable_distributed_tracing: true
EOF
{"ignition":{"config":{},"security":{"tls":{}},"timeouts":{},"version":"2.3.0"},"networkd":{},"passwd":{},"storage":{},"systemd":{"units":[{"dropins":[{"contents":"[Service]\nEnvironment=\"ETCD_IMAGE_TAG=v3.5.1\"\nExecStart=\nExecStart=/usr/lib/coreos/etcd-wrapper $ETCD_OPTS \\\n  --experimental-enable-distributed-tracing=true","name":"20-clct-etcd-member.conf"}],"enable":true,"name":"etcd-member.service"}]}}

The structs were generated by parsing `etcd --help` output and
processing using a go program that I wrote. The program is available
here: https://gist.github.com/jepio/11f447c4f6270e2a23f66d7fa414f3a6.

Signed-off-by: Jeremi Piotrowski <jpiotrowski@microsoft.com>
@jepio jepio requested a review from a team February 1, 2022 15:29
Comment thread config/types/etcd.go
Comment on lines +388 to +390
type Etcd3_4 struct {
Name *string `yaml:"name" cli:"name"`
DataDir *string `yaml:"data_dir" cli:"data-dir"`

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe types could be embedded to deduplicate most of the fields?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

No I don't think that makes sense. I've compared 3.3 to 3.4 and 3.5, every version adds and removes options, and trying to separate that into something that can be embedded will require more maintenance than parsing the options for a specific release and generating a struct from that.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sure, sounds good 👍 I wonder if we could have the code you wrote for generation in go:generate or something.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'll look at that next time we need to update, right now it would be cumbersome to do so as it requires pulling all etcd binaries.

@tormath1

tormath1 commented Feb 1, 2022

Copy link
Copy Markdown
Contributor

thanks for your PR ! While we're here, I'm wondering if we should set the default version to the one Flatcar ships (3.5.0):
https://github.com/flatcar-linux/container-linux-config-transpiler/blob/b55b8c2ce4f47ef6360d4f43997cb5cbcfd8400a/config/types/etcd.go#L32

@jepio

jepio commented Feb 1, 2022

Copy link
Copy Markdown
Member Author

thanks for your PR ! While we're here, I'm wondering if we should set the default version to the one Flatcar ships (3.5.0):

Yeah, I was worried about doing that but I realize now that the default is already defined by the image and the default here only affects yaml parsing and option lookup.

...to match default version deployed by current Flatcar releases.

Signed-off-by: Jeremi Piotrowski <jpiotrowski@microsoft.com>

@tormath1 tormath1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM ! Thanks for addressing this issue. One follow-up step would be to upgrade Mantle dependency consequently to remove warnings in the CI too then.

@jepio jepio merged commit 6e83034 into flatcar-master Feb 8, 2022
@jepio jepio deleted the jepio/etcd-34-35 branch February 8, 2022 15:25
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.

3 participants