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.

Sign up for GitHub

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

feat: add helm chart field #497

Closed
wants to merge 1 commit into from
Closed

feat: add helm chart field #497

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 6 additions & 0 deletions api/v1alpha1/addon_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"k8s.io/utils/ptr"
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"

"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/openapi/patterns"
"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/variables"
)

Expand All @@ -24,6 +25,7 @@ const (
)

type Addons struct {
HelmChartRepository *string `json:"helmChartRepository,omitempty"`
// +optional
CNI *CNI `json:"cni,omitempty"`

Expand All @@ -46,6 +48,10 @@ func (Addons) VariableSchema() clusterv1.VariableSchema {
Description: "Cluster configuration",
Type: "object",
Properties: map[string]clusterv1.JSONSchemaProps{
"helmChartRepository": {
Pattern: patterns.HostAndOptionalPort,
Description: "Optional OCI registry used to pull helm charts for adons",
Copy link
Member

Choose a reason for hiding this comment

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

Is this just an OCI registry or could it also be https helm repository somewhere?

Copy link
Member

Choose a reason for hiding this comment

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

You could expand this description and add example too via Example property.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure what the difference between helm repository and OCI registry is. That entity would need to host all of the charts though. because the repository field for each addon gets set to this value.

Copy link
Member

Choose a reason for hiding this comment

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

Helm repositories used to be supported only accessible via https. OCi support was later introduced meaning charts could be stored and referenced from OCI registries. The only difference is the scheme in the repository URL, using oci:// instead of https://. I think we should support both here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know if all the users are going to be able to push helm charts to a single helm repository im also not sure how to validate it?

Copy link
Member

Choose a reason for hiding this comment

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

This looks like it's only supporting pulling from one helm repository though right for all Helm addons?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, exactly the idea is that this source will have all of the helm charts the user needs

faiq marked this conversation as resolved.
Show resolved Hide resolved
},
"cni": CNI{}.VariableSchema().OpenAPIV3Schema,
"nfd": NFD{}.VariableSchema().OpenAPIV3Schema,
"clusterAutoscaler": ClusterAutoscaler{}.VariableSchema().OpenAPIV3Schema,
Expand Down
2 changes: 2 additions & 0 deletions api/v1alpha1/constants.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,6 @@ const (
AWSVariableName = "aws"
// NutanixVariableName is the Nutanix config patch variable name.
NutanixVariableName = "nutanix"
// HelmChartRepository is the variable name for the OCI registry for addons.
HelmChartRepository = "HelmChartRepository"
)
5 changes: 5 additions & 0 deletions api/v1alpha1/zz_generated.deepcopy.go
View file
Open in desktop

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

1 change: 1 addition & 0 deletions pkg/handlers/generic/lifecycle/clusterautoscaler/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ func (n *DefaultClusterAutoscaler) AfterControlPlaneInitialized(
case v1alpha1.AddonStrategyHelmAddon:
helmChart, err := n.helmChartInfoGetter.For(
ctx,
&req.Cluster,
log,
config.Autoscaler,
)
Expand Down
2 changes: 1 addition & 1 deletion pkg/handlers/generic/lifecycle/cni/calico/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ func (c *CalicoCNI) AfterControlPlaneInitialized(
case v1alpha1.AddonStrategyHelmAddon:
// this is tigera and not calico because we deploy calico via operataor
log.Info("fetching settings for tigera-operator-config")
helmChart, err := c.helmChartInfoGetter.For(ctx, log, config.Tigera)
helmChart, err := c.helmChartInfoGetter.For(ctx, &req.Cluster, log, config.Tigera)
if err != nil {
log.Error(
err,
Expand Down
2 changes: 1 addition & 1 deletion pkg/handlers/generic/lifecycle/cni/cilium/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ func (c *CiliumCNI) AfterControlPlaneInitialized(
client: c.client,
}
case v1alpha1.AddonStrategyHelmAddon:
helmChart, err := c.helmChartInfoGetter.For(ctx, log, config.Cilium)
helmChart, err := c.helmChartInfoGetter.For(ctx, &req.Cluster, log, config.Cilium)
if err != nil {
log.Error(
err,
Expand Down
36 changes: 30 additions & 6 deletions pkg/handlers/generic/lifecycle/config/cm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,12 @@ import (
"gopkg.in/yaml.v2"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"

"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/v1alpha1"
"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/common/pkg/capi/clustertopology/variables"
"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/handlers/generic/clusterconfig"
)

type Component string
Expand All @@ -27,9 +32,10 @@ const (
)

type HelmChartGetter struct {
cl ctrlclient.Reader
cmName string
cmNamespace string
cl ctrlclient.Reader
cmName string
cmNamespace string
variablePath []string
}

type HelmChart struct {
Expand All @@ -46,6 +52,7 @@ func NewHelmChartGetterFromConfigMap(
cl,
cmName,
cmNamespace,
[]string{"addons", v1alpha1.HelmChartRepository},
}
}

Expand All @@ -72,6 +79,7 @@ func (h *HelmChartGetter) get(

func (h *HelmChartGetter) For(
ctx context.Context,
cluster *clusterv1.Cluster,
log logr.Logger,
name Component,
) (*HelmChart, error) {
Expand All @@ -81,6 +89,19 @@ func (h *HelmChartGetter) For(
h.cmName,
h.cmNamespace),
)
varMap := variables.ClusterVariablesToVariablesMap(cluster.Spec.Topology.Variables)

helmChartRepo, found, err := variables.Get[string](
varMap,
clusterconfig.MetaVariableName,
h.variablePath...)
if err != nil {
return nil, fmt.Errorf(
"failed to get helmChartRepo variable from %s: %w",
clusterconfig.MetaVariableName,
err,
)
}
cm, err := h.get(ctx)
if err != nil {
return nil, err
Expand All @@ -89,7 +110,10 @@ func (h *HelmChartGetter) For(
if !ok {
return nil, fmt.Errorf("did not find key %s in %v", name, cm.Data)
}
var settings HelmChart
err = yaml.Unmarshal([]byte(d), &settings)
return &settings, err
var chart HelmChart
err = yaml.Unmarshal([]byte(d), &chart)
supershal marked this conversation as resolved.
Show resolved Hide resolved
if found {
chart.Repository = helmChartRepo
}
return &chart, err
}
9 changes: 7 additions & 2 deletions pkg/handlers/generic/lifecycle/csi/nutanix-csi/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ func (n *NutanixCSI) handleHelmAddonApply(
"cluster",
ctrlclient.ObjectKeyFromObject(&req.Cluster),
)
helmChart, err := n.helmChartInfoGetter.For(ctx, log, config.NutanixStorageCSI)
helmChart, err := n.helmChartInfoGetter.For(ctx, &req.Cluster, log, config.NutanixStorageCSI)
if err != nil {
return fmt.Errorf("failed to get values for nutanix-csi-config %w", err)
}
Expand Down Expand Up @@ -186,7 +186,12 @@ func (n *NutanixCSI) handleHelmAddonApply(
return fmt.Errorf("failed to apply nutanix-csi installation HelmChartProxy: %w", err)
}

snapshotHelmChart, err := n.helmChartInfoGetter.For(ctx, log, config.NutanixSnapshotCSI)
snapshotHelmChart, err := n.helmChartInfoGetter.For(
ctx,
&req.Cluster,
log,
config.NutanixSnapshotCSI,
)
if err != nil {
return fmt.Errorf("failed to get values for nutanix-csi-config %w", err)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/handlers/generic/lifecycle/nfd/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func (n *DefaultNFD) AfterControlPlaneInitialized(
client: n.client,
}
case v1alpha1.AddonStrategyHelmAddon:
helmChart, err := n.helmChartInfoGetter.For(ctx, log, config.NFD)
helmChart, err := n.helmChartInfoGetter.For(ctx, &req.Cluster, log, config.NFD)
if err != nil {
log.Error(
err,
Expand Down
Loading