Add console subchart in redpanda helm chart #1558
Conversation
RafalKorepta
force-pushed
the
rk/k8s-305/console-subchart-in-redpanda
branch
2 times, most recently
from
3804758
to
8c03e67
Compare
RafalKorepta
force-pushed
the
rk/k8s-305/console-subchart-in-redpanda
branch
from
8c03e67
to
732abdd
Compare
charts/redpanda/chart_test.go
Outdated
| values.Console = &redpanda.PartialConsole{Enabled: ptr.To(false)} | ||
| values.Console = &console.PartialValues{ | ||
| Enabled: ptr.To(true), | ||
| Secret: &console.PartialSecretConfig{ |
There was a problem hiding this comment.
Can you add a note about why the secret is set here? At first I thought it was just being copied but then I realized we need it to be consistent cross both helm and go invocations.
There was a problem hiding this comment.
In the first implementation secret was always generated. Now as License is added it makes sense to have JWT placeholder for stable tests.
Do you need a one line comment that states "For generating stable test result"?
|
|
||
| consoleDot := dot.Subcharts["console"] | ||
|
|
||
| consoleValue := helmette.UnmarshalInto[console.Values](consoleDot.Values) |
There was a problem hiding this comment.
Let's add a lot of comments to this function. There were a handful previously but we can definitely do better.
Noting that the console.* section of the redpanda chart disables the secret/configmap in order for the redpanda chart to render it with connection information will be a great start.
charts/redpanda/console.tpl.go
Outdated
| if cert.SecretRef != nil { | ||
| secretName = cert.SecretRef.Name | ||
| } | ||
| if cert.CAEnabled { |
There was a problem hiding this comment.
You can either solve this here or file a ticket handle it later: truststores (Defined on the listener instead of the cert) need to be handled here as they can also be used to set the CA.
There was a problem hiding this comment.
I think the Volumes and VolumeMounts have now much more clear functions.
| Mode: mode, | ||
| BuildFlags: []string{"-tags=generate"}, | ||
| Dir: cwd, | ||
| Mode: mode, |
There was a problem hiding this comment.
Is this change intentional?
There was a problem hiding this comment.
It is intentional as Redpanda.Values have now console.PartialValues. As the PartialValues struct is in the file that have generated build tag, the load package needs that.
There was a problem hiding this comment.
Should we also remove the build directive from the generation as nothing will be using that now?
RafalKorepta
force-pushed
the
rk/k8s-305/console-subchart-in-redpanda
branch
7 times, most recently
from
1b39385
to
ac9795e
Compare
The Console partial values are used for pragmatic reasons as genpartial does not convert console.Values to console.PartialValues inside Redpanda Values partial generation. In both structs the type is the same. Unit tests that marshals Redpanda partial values into file does not work as expected. The console part of the marshalled values have multiple empty strings or nulls. It is caused by the fact omitempty is not always set in Console.Values. In normal template evaluation those nulls and empty strings would be replaced by default values from values.yaml file. To unblock operator integration Console.PartialValues is used, until genpartail is smart enough.
RafalKorepta
force-pushed
the
rk/k8s-305/console-subchart-in-redpanda
branch
from
ac9795e
to
83fd674
Compare
This PR is based on top of #1557
caedc4a
Support full Console volumes
The Console partial values are used for pragmatic reasons as genpartial does
not convert console.Values to console.PartialValues inside Redpanda Values
partial generation. In both structs the type is the same. Unit tests that
marshals Redpanda partial values into file does not work as expected. The
console part of the marshalled values have multiple empty strings or nulls.
It is caused by the fact omitempty is not always set in Console.Values. In
normal template evaluation those nulls and empty strings would be replaced by
default values from values.yaml file. To unblock operator integration
Console.PartialValues is used, until genpartail is smart enough.
598dfe9
Add Console as subchart in Redpanda chart
ac9795e
Use certification path const in all places