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

[Proposals] Personal take on improvements #136

Open
Al-Pragliola opened this issue Sep 20, 2024 · 4 comments
Open

[Proposals] Personal take on improvements #136

Al-Pragliola opened this issue Sep 20, 2024 · 4 comments

Comments

@Al-Pragliola
Copy link
Contributor

Make use of Viper

Viper is setup in internal/controller/defaults.go

func init() {
	// init viper for config env variables
	viper.AutomaticEnv()
}

we can make use of it when taking configuration parameters from environment variables.

Switch from template files to actual entity types

I see the nice pattern of using template files to create different resources from the controller, but in my opinion it can be a double edged sword because we use the yaml package to map the templates to the kubernetes resource types and by default go if there's a mismatch with fields just sets the default value for that type without raising an error, creating a possible bug that is quite difficult to catch. If we use the go types to create the resources we lose the "nice to read" template but we have a 1:1 map from type to kubernetes applied resource.

Split large files

We should try to split the files that are close to 1k~ lines into several smaller files (https://google.github.io/styleguide/go/best-practices.html#package-size) to increase readability and reduce cognitive load.

Add more logs

We should add more logs to make debugging easier

Add linting

We should add linting steps, so to have and external help to avoid common mistakes and improve code quality

E2e tests ( envtest )

We should add e2e/envtest tests to avoid regressions and check that the operator is doing what is supposed to do

@tarilabs
Copy link
Member

thank you @Al-Pragliola , I like especially about the e2e testing, I can see there are some already, but if those could be extended also to cover for non-regression in some "use-cases" or scenarios, I believe would be awesome.

@rareddy
Copy link
Contributor

rareddy commented Sep 24, 2024

on template file usage I thought this was recommended pattern by operator SDK group to reduce the development time.

I also remember we advocated for linting from beginning. did we only use in rest server ? :)

+100 more e2e tests

@Al-Pragliola
Copy link
Contributor Author

on template file usage I thought this was recommended pattern by operator SDK group to reduce the development time.

I didn't know about this as a recommended pattern, do you have any resource that I can read on the topic?

I also remember we advocated for linting from beginning. did we only use in rest server ? :)

I don't see any reference to linting in the CI or the Makefile's targets

+100 more e2e tests

yay I'm willing to work on that

@dhirajsb
Copy link
Contributor

I didn't know about this as a recommended pattern, do you have any resource that I can read on the topic?

Platform operator also uses templates heavily to minimize the development time and maintenance on resource configs. I'm not sure whether there is any documentation about it or not.

Templates also keep the source resource format close to the target form, which helps diagnose issues quickly. Instead of having to sort of reverse engineer what the target yaml resource will look like from go source. It also makes it accessible to non-go or new go programmers.

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

No branches or pull requests

4 participants