Migrate go code from helm-chart to redpanda-operator #334
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
To correctly handle negative numbers that are parsed as `ast.UnaryExpr` the transplier will past `Literal`.
To handle Redpanda configuration in go code the `first` function needs to be included to helmette as part of the sprig definition. Negative numbers are recognized as `ast.UnaryExpr` with sub token. Now they can be transpiled. P.S. The `[]byte` array can be represented only in Go, but not in template.
Go template supports great or equal and less or equal with different name 'ge' and 'le' respectively. Reference * https://pkg.go.dev/text/template * https://github.com/golang/go/blob/62711d61e5ba3a6c42bc57a28c2402dd8669261f/src/text/template/doc.go#L380-L394 * https://github.com/golang/go/blob/62711d61e5ba3a6c42bc57a28c2402dd8669261f/src/text/template/funcs.go#L55-L61
Prior to this commit, there were no test cases that exercised any of the redpanda version check helpers that are littered throughout the chart. Given a handful of difficulties with the first attempt of migrating said helpers to go, this commit adds in a matrix of regression tests to ensure that the conversion either preserves existing behavior or fixes any latent bugs.
`toString` function is required for Redpanda configuration conversion. Reference https://github.com/redpanda-data/helm-charts/blob/d9bc9dbf95d54f79541df812be0d17dce5d2f8fb/charts/redpanda/templates/_configmap.tpl#L380 https://github.com/redpanda-data/helm-charts/blob/d9bc9dbf95d54f79541df812be0d17dce5d2f8fb/charts/redpanda/templates/_configmap.tpl#L382
Min function is required for config map conversion. Reference https://github.com/redpanda-data/helm-charts/blob/66725937f7bbf030d96a5e52230c660b4409f50f/charts/redpanda/templates/_configmap.tpl#L76
`trimSuffix` function is required for Redpanda configuration conversion. Reference https://github.com/redpanda-data/helm-charts/blob/d9bc9dbf95d54f79541df812be0d17dce5d2f8fb/charts/redpanda/templates/_helpers.tpl#L169
In commit 82a7e51, a typo was accidentally introduced that broke the connectors integration of the redpanda chart. This issue went unnoticed until discovered by an end user due to a lack of any test case that specified `connectors.enabled=true`. This commit corrects the typo and adds a minimal regression test. A manual check was also performed to ensure that the behavior of `connectors.enabled=true` is the same prior to `82a7e519af7ad9591a5f55ac245022e0886ba22b` Fixes redpanda-data/helm-charts#1321
Previously, gotohelm could panic or generate in valid templates in certain cases when encountering assignment to embedded fields in either the shorted or elongated form as the selector tranpilation case did not correctly handle embedding. This commit update `getFields` to instead returned a flattened list of all fields, including ones inherited from embedded structs. It also updates the `Selector` AST to elide itself if it is accessing an embedded field. As a side effect, this has removed the need to generate nested groups of `mustMergeOverwrite` when calculating the zero values of structs with embedded fields.
This commit converts the servicemonitor.yaml template to a golang equivalent. Notably, the default value for `monitoring.tlsConfig` has changed to `null` to make the gocode more ergonomic.
Previously, gotohelm required special casing for functions that returned `(T, error)` as helm/sprig functions have no notion of returning errors. This commit adds automatic detection for such cases. Any function annotated with `+gotohelm:builtin=` that returns `(T, error)` will automatically be wrapped with `list nil` to simulate a successful return.
Prior to this commit, the semver regression test suite was accidentally performing tests soley against v24.1.0 due taking the address of a loop variable. This commit appropriately "captures" the `version` variable and adds in support for asserting on failures of `helm template. See also https://go.dev/blog/loopvar-preview
This commit converts the collection of redpanda version gate functions from helm to go.
Without this, it ends up on the same depth as `matchLabels:` in my testing:
podAntiAffinity:
preferredDuringSchedulingIgnoredDuringExecution:
- weight: 100
podAffinityTerm:
topologyKey: kubernetes.io/hostname
labelSelector:
matchLabels:
app.kubernetes.io/component: redpanda-statefulset
app.kubernetes.io/instance: redpanda
app.kubernetes.io/name: redpanda
This commit migrates `common-volumes`, `default-volumes`, `default-mounts`, and `common-mounts` to go.
Prior to this commit, the redpanda helm chart would almost always generate `Certificates`, even if they we're not being used. This can be frustrating for users that are not using cert-manager or want to manage the integration themselves. This commit makes it easier to disable most of the certificates that the chart generates by fixing a mistranslated boolean check, respecting the `enabled` field on elements of `tls.certs`, and providing a few example test cases.
The `toYaml` function is not part of the sprig library, but helm provides it. The `toYaml` is required for Redpanda configuration conversion to go. Referance https://github.com/redpanda-data/helm-charts/blob/d9bc9dbf95d54f79541df812be0d17dce5d2f8fb/charts/redpanda/templates/_configmap.tpl#L69 https://github.com/redpanda-data/helm-charts/blob/d9bc9dbf95d54f79541df812be0d17dce5d2f8fb/charts/redpanda/templates/_configmap.tpl#L79 https://github.com/redpanda-data/helm-charts/blob/d9bc9dbf95d54f79541df812be0d17dce5d2f8fb/charts/redpanda/templates/_configmap.tpl#L157 helm function definition https://github.com/helm/helm/blob/e90b456d655e78d7c72a32a52a9b70bc1984c33f/pkg/engine/funcs.go#L49-L68
Based on broker configuration template https://docs.redpanda.com/23.3/reference/node-configuration-sample/ the `cluster-id` and `organization` was defined at the top level. It should be defined under `redpanda` part of the `redpanda.yaml`. Based on redpanda-data/redpanda#9713 `organization` is being removed from the cluster configuration.
Floating point constants in go, when possible, are represented as division of rational numbers. For example, 0.1 would be represented as 1/10. This syntax is passed directly to helm which does not support such division. This commit handles floating point constants and forcibly represents them as decimals in all cases. Refresh of gotohelm-float-consts
Prior to this commit, writing gotohelm code need to be very tactical when dealing with numeric values. This lead to type casts being littered around the codebase and likely a handful of unnoticed bugs. This commit upgrades the transpiler to inject type casts into transpiled code at function boundaries and struct field references. While this solution is not perfect, it should dramatically reduce needless type casting and make code slightly more idiomatic.
This commit adds the `AsNumeric` and `AsIntegral` helper functions to the `helmette` package to aid working with numeric values in gotohelm code. A future commit will disallow type assertions and type checks of numeric values in favor of these helpers as they are the closest approximation to keeping helm and go behavior the same.
Prior to this commit is was value to use type checks or assertions within gotohelm code with numeric values. For example: `x.(int)`. Due to the numerous limitations of go templates and helm, it's not feasible to fully support such expressions as there's no type information that could be used to distinguish between ints, int64s, and float64s. This commit causes the transpiler to fail if it encounters such a statement and instead points users to the recently added `AsNumeric` and `AsIntegral` functions.
Instance methods are transpiled into templates that receives as a first argument the instance of the struct. Any function call that have pointer receiver would be translated to template `include` statement with one argument which is struct itself. There is special case for Values.AsMap method as it is not defined as seperated template, but rather it is build in feature of the helm engine. In future improvements instance methods transpilation could support arguments.
Previously the node reference might point to the rewritten code. That might be
hard to find the file where the error comes from. This commit introduces a file
path, line number and offset.
Previous:
```
Failed to transpile "redpanda": unsupported ast.Node: *ast.CallExpr
type checks on numeric types are unreliable due to JSON casting all numbers to float64's. Instead use `helmette.IsNumeric` or `helmette.AsIntegral
helmette.TypeTest[float64](v)
```
New:
```
Failed to transpile "redpanda": unsupported ast.Node: *ast.CallExpr
type checks on numeric types are unreliable due to JSON casting all numbers to float64's. Instead use `helmette.IsNumeric` or `helmette.AsIntegral`
/Users/rafalkorepta/workspace/redpanda/helm-charts/charts/redpanda/values.go:864:37
helmette.TypeTest[float64](v)
```
This commit adds `gofumpt` and `staticcheck` to our CI's lint task as well as fixing any issues initially found by both.
Error from go `packages.Load` function returns an error when patterns was
invalid as defined by the underlying build system. Errors associated with
a particular package are recorded in the corresponding Package's Errors list.
```
=== RUN TestLoadPackages
--- FAIL: TestLoadPackages (0.59s)
panic: pkg errors (/Users/rafalkorepta/workspace/redpanda/helm-charts/pkg/gotohelm/testdata/src/example/astrewrites/astrewrites.go:4:9: could not import k8s.io/api/core/v1 (invalid package name: "")): 1:9: expected 'EOF', found 'type' [recovered]
panic: pkg errors (/Users/rafalkorepta/workspace/redpanda/helm-charts/pkg/gotohelm/testdata/src/example/astrewrites/astrewrites.go:4:9: could not import k8s.io/api/core/v1 (invalid package name: "")): 1:9: expected 'EOF', found 'type' [recovered]
panic: pkg errors (/Users/rafalkorepta/workspace/redpanda/helm-charts/pkg/gotohelm/testdata/src/example/astrewrites/astrewrites.go:4:9: could not import k8s.io/api/core/v1 (invalid package name: "")): 1:9: expected 'EOF', found 'type'
goroutine 1278 [running]:
testing.tRunner.func1.2({0x106431100, 0x1401c250f90})
/opt/homebrew/opt/go/libexec/src/testing/testing.go:1631 +0x1c4
testing.tRunner.func1()
/opt/homebrew/opt/go/libexec/src/testing/testing.go:1634 +0x33c
panic({0x106431100?, 0x1401c250f90?})
/opt/homebrew/opt/go/libexec/src/runtime/panic.go:770 +0x124
golang.org/x/tools/go/ast/astutil.Apply.func1()
/Users/rafalkorepta/go/pkg/mod/golang.org/x/[email protected]/go/ast/astutil/rewrite.go:46 +0xb8
panic({0x106431100?, 0x1401c250f90?})
/opt/homebrew/opt/go/libexec/src/runtime/panic.go:770 +0x124
github.com/redpanda-data/helm-charts/pkg/gotohelm.typeToNode(0x1400079b800, {0x1066c8d78, 0x10733ef40})
/Users/rafalkorepta/workspace/redpanda/helm-charts/pkg/gotohelm/rewrite.go:110 +0x114
github.com/redpanda-data/helm-charts/pkg/gotohelm.rewriteMultiValueSyntaxToHelpers.func2(0x1401c267330)
/Users/rafalkorepta/workspace/redpanda/helm-charts/pkg/gotohelm/rewrite.go:271 +0x208
golang.org/x/tools/go/ast/astutil.(*application).apply(0x1401c267320, {0x1066c9048?, 0x140066a5680?}, {0x1060d29b4?, 0x0?}, 0x0?, {0x1066c90c0?, 0x14006470ed0?})
/Users/rafalkorepta/go/pkg/mod/golang.org/x/[email protected]/go/ast/astutil/rewrite.go:197 +0x160
golang.org/x/tools/go/ast/astutil.(*application).applyList(0x1401c267320, {0x1066c9048, 0x140066a5680}, {0x1060d29b4, 0x3})
/Users/rafalkorepta/go/pkg/mod/golang.org/x/[email protected]/go/ast/astutil/rewrite.go:482 +0x78
golang.org/x/tools/go/ast/astutil.(*application).apply(0x1401c267320, {0x1066c9688?, 0x14006470f00?}, {0x1060d3088?, 0x0?}, 0x0?, {0x1066c9048?, 0x140066a5680?})
/Users/rafalkorepta/go/pkg/mod/golang.org/x/[email protected]/go/ast/astutil/rewrite.go:336 +0x890
golang.org/x/tools/go/ast/astutil.(*application).applyList(0x1401c267320, {0x1066c9688, 0x14006470f00}, {0x1060d3088, 0x4})
/Users/rafalkorepta/go/pkg/mod/golang.org/x/[email protected]/go/ast/astutil/rewrite.go:482 +0x78
golang.org/x/tools/go/ast/astutil.(*application).apply(0x1401c267320, {0x1066c9688?, 0x14006470f30?}, {0x1060d3088?, 0x18?}, 0x1401c22b400?, {0x1066c9688?, 0x14006470f00?})
/Users/rafalkorepta/go/pkg/mod/golang.org/x/[email protected]/go/ast/astutil/rewrite.go:351 +0x91c
golang.org/x/tools/go/ast/astutil.(*application).applyList(0x1401c267320, {0x1066c9688, 0x14006470f30}, {0x1060d3088, 0x4})
/Users/rafalkorepta/go/pkg/mod/golang.org/x/[email protected]/go/ast/astutil/rewrite.go:482 +0x78
golang.org/x/tools/go/ast/astutil.(*application).apply(0x1401c267320, {0x1066c8f58?, 0x14006470f60?}, {0x1060d3014?, 0x14000f678c8?}, 0x105046030?, {0x1066c9688?, 0x14006470f30?})
/Users/rafalkorepta/go/pkg/mod/golang.org/x/[email protected]/go/ast/astutil/rewrite.go:351 +0x91c
golang.org/x/tools/go/ast/astutil.(*application).apply(0x1401c267320, {0x1066c8e68?, 0x14000963ae0?}, {0x1060d3c3e?, 0x14000963ae0?}, 0x1060d3c3e?, {0x1066c8f58?, 0x14006470f60?})
/Users/rafalkorepta/go/pkg/mod/golang.org/x/[email protected]/go/ast/astutil/rewrite.go:427 +0x10cc
golang.org/x/tools/go/ast/astutil.(*application).applyList(0x1401c267320, {0x1066c8e68, 0x14000963ae0}, {0x1060d3c3e, 0x5})
/Users/rafalkorepta/go/pkg/mod/golang.org/x/[email protected]/go/ast/astutil/rewrite.go:482 +0x78
golang.org/x/tools/go/ast/astutil.(*application).apply(0x1401c267320, {0x1066c9c50?, 0x1401c250ea0?}, {0x1060d3084?, 0x18?}, 0x14000268008?, {0x1066c8e68?, 0x14000963ae0?})
/Users/rafalkorepta/go/pkg/mod/golang.org/x/[email protected]/go/ast/astutil/rewrite.go:433 +0x46c
golang.org/x/tools/go/ast/astutil.Apply({0x1066c8e68, 0x14000963ae0}, 0x1401c270330, 0x0)
/Users/rafalkorepta/go/pkg/mod/golang.org/x/[email protected]/go/ast/astutil/rewrite.go:51 +0x10c
github.com/redpanda-data/helm-charts/pkg/gotohelm.rewriteMultiValueSyntaxToHelpers(0x1400079b800, 0x14000963ae0)
/Users/rafalkorepta/workspace/redpanda/helm-charts/pkg/gotohelm/rewrite.go:247 +0xf0
github.com/redpanda-data/helm-charts/pkg/gotohelm.LoadPackages(0x14000df7e90, {0x14001fcf930, 0x1, 0x1})
/Users/rafalkorepta/workspace/redpanda/helm-charts/pkg/gotohelm/rewrite.go:60 +0x29c
github.com/redpanda-data/helm-charts/pkg/gotohelm.TestLoadPackages(0x14001996680)
/Users/rafalkorepta/workspace/redpanda/helm-charts/pkg/gotohelm/rewrite_test.go:19 +0x1d8
testing.tRunner(0x14001996680, 0x1066bbef0)
/opt/homebrew/opt/go/libexec/src/testing/testing.go:1689 +0xec
created by testing.(*T).Run in goroutine 1
/opt/homebrew/opt/go/libexec/src/testing/testing.go:1742 +0x318
Process finished with the exit code 1
```
Prior to this commit the operator chart's declared permissions had grown out of sync with those declared by the operator itself. This commit re-synchronizes the permissions, adds in regression tests to prevent such drift from happening again, and releases the updated chart. Co-authored-by: Rafal Korepta <[email protected]>
Prior to this commit gotohelm did not support first class functions. This made certain operations tedious to implement as cases would have to be manually copied. This commit adds support for passing functions as arguments and invoking said functions. Closures themselves are not supported.
Prior to this commit the `podTemplate` value only permitted overriding a few special cased fields. This commit makes the implementation generic enough to allow: - Adding additional containers - Adding additional init containers - Adding additional environment variables to _any_ container - overriding any value on podSpec. In particular, this commit is aimed at allowing users to specify `priority` or `priorityClassName` on the redpanda statefulset to prevent undesired evictions. K8S-423
RafalKorepta
force-pushed
the
rk/move-import-to-operator-repo
branch
from
3da6a03 to
d29846b
Compare
chrisseto
changed the title
RafalKorepta
force-pushed
the
rk/move-import-to-operator-repo
branch
from
d29846b to
0c39f48
Compare
david-yu
approved these changes
Dec 3, 2024
RafalKorepta
force-pushed
the
rk/move-import-to-operator-repo
branch
from
0c39f48 to
320afec
Compare
RafalKorepta
force-pushed
the
rk/move-import-to-operator-repo
branch
from
320afec to
eede16a
Compare
RafalKorepta
force-pushed
the
rk/move-import-to-operator-repo
branch
from
e2d8047 to
a4513e8
Compare
|
Due to issues with Github after force push new PR was opened #339 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Based on #289
All go imports are moved from
github.com/redpanda-data/helm-chartstogithub.aaakk.us.kg/redpanda-data/redpanda-operator.K8S-444