Skip to content

Commit 0a3b6c6

Browse files
jimmidysonjimmidyson
committed
fix: Handle long cluster names
This commit starts fixing a bug that means addons fail to be fully deployed if the cluster name is longer than 44 characters. This is caused by the name of the HCP being over 63 characters. This name is then used in HRP labels which have a maximum length of 63 characters, so the HRPs are rejected by the API server when CAAPH applies them. The fix is to use generate name and labels on the HCP to ensure uniqueness by lookup rather than by using a deterministic name.
1 parent bd575b7 commit 0a3b6c6

File tree

17 files changed

+218
-405
lines changed

17 files changed

+218
-405
lines changed

‎pkg/handlers/generic/lifecycle/addons/helmaddon.go

Lines changed: 61 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20,22 +20,27 @@ import (
2020
handlersutils "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/handlers/utils"
2121
)
2222

23+
var (
24+
HelmReleaseNameHashLabel = "addons.cluster.x-k8s.io/helm-release-name-hash"
25+
ClusterNamespaceLabel = clusterv1.ClusterNamespaceAnnotation
26+
)
27+
2328
type HelmAddonConfig struct {
2429
defaultValuesTemplateConfigMapName string
2530

26-
defaultHelmReleaseNamespace string
27-
defaultHelmReleaseName string
31+
helmReleaseNamespace string
32+
helmReleaseNamePrefix string
2833
}
2934

3035
func NewHelmAddonConfig(
3136
defaultValuesTemplateConfigMapName string,
32-
defaultHelmReleaseNamespace string,
33-
defaultHelmReleaseName string,
37+
helmReleaseNamespace string,
38+
helmReleaseNamePrefix string,
3439
) *HelmAddonConfig {
3540
return &HelmAddonConfig{
3641
defaultValuesTemplateConfigMapName: defaultValuesTemplateConfigMapName,
37-
defaultHelmReleaseNamespace: defaultHelmReleaseNamespace,
38-
defaultHelmReleaseName: defaultHelmReleaseName,
42+
helmReleaseNamespace: helmReleaseNamespace,
43+
helmReleaseNamePrefix: helmReleaseNamePrefix,
3944
}
4045
}
4146

@@ -52,6 +57,7 @@ type helmAddonApplier struct {
5257
config *HelmAddonConfig
5358
client ctrlclient.Client
5459
helmChart *lifecycleconfig.HelmChart
60+
opts []applyOption
5561
}
5662

5763
var _ Applier = &helmAddonApplier{}
@@ -68,12 +74,42 @@ func NewHelmAddonApplier(
6874
}
6975
}
7076

77+
type applyOptions struct {
78+
valueTemplater func(cluster *clusterv1.Cluster, valuesTemplate string) (string, error)
79+
targetCluster *clusterv1.Cluster
80+
}
81+
82+
type applyOption func(*applyOptions)
83+
84+
func (a *helmAddonApplier) WithValueTemplater(
85+
valueTemplater func(cluster *clusterv1.Cluster, valuesTemplate string) (string, error),
86+
) *helmAddonApplier {
87+
a.opts = append(a.opts, func(o *applyOptions) {
88+
o.valueTemplater = valueTemplater
89+
})
90+
91+
return a
92+
}
93+
94+
func (a *helmAddonApplier) WithTargetCluster(cluster *clusterv1.Cluster) *helmAddonApplier {
95+
a.opts = append(a.opts, func(o *applyOptions) {
96+
o.targetCluster = cluster
97+
})
98+
99+
return a
100+
}
101+
71102
func (a *helmAddonApplier) Apply(
72103
ctx context.Context,
73104
cluster *clusterv1.Cluster,
74105
defaultsNamespace string,
75106
log logr.Logger,
76107
) error {
108+
applyOpts := &applyOptions{}
109+
for _, opt := range a.opts {
110+
opt(applyOpts)
111+
}
112+
77113
log.Info("Retrieving installation values template for cluster")
78114
values, err := handlersutils.RetrieveValuesTemplate(
79115
ctx,
@@ -88,29 +124,42 @@ func (a *helmAddonApplier) Apply(
88124
)
89125
}
90126

127+
if applyOpts.valueTemplater != nil {
128+
values, err = applyOpts.valueTemplater(cluster, values)
129+
if err != nil {
130+
return fmt.Errorf("failed to template Helm values: %w", err)
131+
}
132+
}
133+
134+
targetCluster := cluster
135+
if applyOpts.targetCluster != nil {
136+
targetCluster = applyOpts.targetCluster
137+
}
138+
91139
chartProxy := &caaphv1.HelmChartProxy{
92140
TypeMeta: metav1.TypeMeta{
93141
APIVersion: caaphv1.GroupVersion.String(),
94142
Kind: "HelmChartProxy",
95143
},
96144
ObjectMeta: metav1.ObjectMeta{
97-
Namespace: cluster.Namespace,
98-
Name: a.config.defaultHelmReleaseName + "-" + cluster.Name,
145+
Namespace: targetCluster.Namespace,
146+
Name: fmt.Sprintf("%s-%s", a.config.helmReleaseNamePrefix, cluster.UID),
99147
},
100148
Spec: caaphv1.HelmChartProxySpec{
101149
RepoURL: a.helmChart.Repository,
102150
ChartName: a.helmChart.Name,
103151
ClusterSelector: metav1.LabelSelector{
104-
MatchLabels: map[string]string{clusterv1.ClusterNameLabel: cluster.Name},
152+
MatchLabels: map[string]string{clusterv1.ClusterNameLabel: targetCluster.Name},
105153
},
106-
ReleaseNamespace: a.config.defaultHelmReleaseNamespace,
107-
ReleaseName: a.config.defaultHelmReleaseName,
154+
ReleaseNamespace: a.config.helmReleaseNamespace,
155+
ReleaseName: a.config.helmReleaseNamePrefix,
108156
Version: a.helmChart.Version,
109157
ValuesTemplate: values,
110158
},
111159
}
160+
112161
handlersutils.SetTLSConfigForHelmChartProxyIfNeeded(chartProxy)
113-
if err = controllerutil.SetOwnerReference(cluster, chartProxy, a.client.Scheme()); err != nil {
162+
if err = controllerutil.SetOwnerReference(targetCluster, chartProxy, a.client.Scheme()); err != nil {
114163
return fmt.Errorf(
115164
"failed to set owner reference on HelmChartProxy %q: %w",
116165
chartProxy.Name,

‎pkg/handlers/generic/lifecycle/ccm/aws/handler.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ const (
2424
awsCCMPrefix = "aws-ccm-"
2525

2626
defaultHelmReleaseNamespace = "kube-system"
27-
defaultHelmReleaseName = "aws-cloud-controller-manager"
27+
defaultHelmReleaseName = "aws-ccm"
2828
)
2929

3030
type AWSCCMConfig struct {

‎pkg/handlers/generic/lifecycle/ccm/nutanix/handler.go

Lines changed: 43 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,11 @@ import (
1212

1313
"github.com/go-logr/logr"
1414
"github.com/spf13/pflag"
15-
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1615
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
1716
ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
18-
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
1917

20-
caaphv1 "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/external/sigs.k8s.io/cluster-api-addon-provider-helm/api/v1alpha1"
2118
apivariables "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/api/variables"
22-
"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/common/pkg/k8s/client"
19+
"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/handlers/generic/lifecycle/addons"
2320
"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/handlers/generic/lifecycle/config"
2421
"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/handlers/options"
2522
handlersutils "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/handlers/utils"
@@ -80,25 +77,11 @@ func (p *provider) Apply(
8077
return ErrMissingCredentials
8178
}
8279

83-
log.Info("Retrieving Nutanix CCM installation values template for cluster")
84-
values, err := handlersutils.RetrieveValuesTemplate(
85-
ctx,
86-
p.client,
87-
p.config.defaultValuesTemplateConfigMapName,
88-
p.config.DefaultsNamespace(),
89-
)
90-
if err != nil {
91-
return fmt.Errorf(
92-
"failed to retrieve Nutanix CCM installation values template for cluster: %w",
93-
err,
94-
)
95-
}
96-
9780
// It's possible to have the credentials Secret be created by the Helm chart.
9881
// However, that would leave the credentials visible in the HelmChartProxy.
9982
// Instead, we'll create the Secret on the remote cluster and reference it in the Helm values.
10083
if clusterConfig.Addons.CCM.Credentials != nil {
101-
err = handlersutils.EnsureOwnerReferenceForSecret(
84+
err := handlersutils.EnsureOwnerReferenceForSecret(
10285
ctx,
10386
p.client,
10487
clusterConfig.Addons.CCM.Credentials.SecretRef.Name,
@@ -134,77 +117,56 @@ func (p *provider) Apply(
134117
return fmt.Errorf("failed to get values for nutanix-ccm-config: %w", err)
135118
}
136119

137-
// The configMap will contain the Helm values, but templated with fields that need to be filled in.
138-
values, err = templateValues(clusterConfig, values)
139-
if err != nil {
140-
return fmt.Errorf("failed to template Helm values read from ConfigMap: %w", err)
141-
}
142-
143-
hcp := &caaphv1.HelmChartProxy{
144-
TypeMeta: metav1.TypeMeta{
145-
APIVersion: caaphv1.GroupVersion.String(),
146-
Kind: "HelmChartProxy",
147-
},
148-
ObjectMeta: metav1.ObjectMeta{
149-
Namespace: cluster.Namespace,
150-
Name: "nutanix-ccm-" + cluster.Name,
151-
},
152-
Spec: caaphv1.HelmChartProxySpec{
153-
RepoURL: helmChart.Repository,
154-
ChartName: helmChart.Name,
155-
ClusterSelector: metav1.LabelSelector{
156-
MatchLabels: map[string]string{clusterv1.ClusterNameLabel: cluster.Name},
157-
},
158-
ReleaseNamespace: defaultHelmReleaseNamespace,
159-
ReleaseName: defaultHelmReleaseName,
160-
Version: helmChart.Version,
161-
ValuesTemplate: values,
162-
},
163-
}
164-
handlersutils.SetTLSConfigForHelmChartProxyIfNeeded(hcp)
165-
if err = controllerutil.SetOwnerReference(cluster, hcp, p.client.Scheme()); err != nil {
166-
return fmt.Errorf(
167-
"failed to set owner reference on nutanix-ccm installation HelmChartProxy: %w",
168-
err,
169-
)
170-
}
120+
applier := addons.NewHelmAddonApplier(
121+
addons.NewHelmAddonConfig(
122+
p.config.defaultValuesTemplateConfigMapName,
123+
defaultHelmReleaseNamespace,
124+
defaultHelmReleaseName,
125+
),
126+
p.client,
127+
helmChart,
128+
).WithValueTemplater(templateValuesFunc(clusterConfig))
171129

172-
if err = client.ServerSideApply(ctx, p.client, hcp, client.ForceOwnership); err != nil {
130+
if err = applier.Apply(ctx, cluster, p.config.DefaultsNamespace(), log); err != nil {
173131
return fmt.Errorf("failed to apply nutanix-ccm installation HelmChartProxy: %w", err)
174132
}
175133

176134
return nil
177135
}
178136

179-
func templateValues(clusterConfig *apivariables.ClusterConfigSpec, text string) (string, error) {
180-
helmValuesTemplate, err := template.New("").Parse(text)
181-
if err != nil {
182-
return "", fmt.Errorf("failed to parse Helm values template: %w", err)
183-
}
137+
func templateValuesFunc(
138+
clusterConfig *apivariables.ClusterConfigSpec,
139+
) func(*clusterv1.Cluster, string) (string, error) {
140+
return func(_ *clusterv1.Cluster, valuesTemplate string) (string, error) {
141+
helmValuesTemplate, err := template.New("").Parse(valuesTemplate)
142+
if err != nil {
143+
return "", fmt.Errorf("failed to parse Helm values template: %w", err)
144+
}
184145

185-
type input struct {
186-
PrismCentralHost string
187-
PrismCentralPort int32
188-
PrismCentralInsecure bool
189-
PrismCentralAdditionalTrustBundle string
190-
}
146+
type input struct {
147+
PrismCentralHost string
148+
PrismCentralPort int32
149+
PrismCentralInsecure bool
150+
PrismCentralAdditionalTrustBundle string
151+
}
191152

192-
address, port, err := clusterConfig.Nutanix.PrismCentralEndpoint.ParseURL()
193-
if err != nil {
194-
return "", err
195-
}
196-
templateInput := input{
197-
PrismCentralHost: address,
198-
PrismCentralPort: port,
199-
PrismCentralInsecure: clusterConfig.Nutanix.PrismCentralEndpoint.Insecure,
200-
PrismCentralAdditionalTrustBundle: clusterConfig.Nutanix.PrismCentralEndpoint.AdditionalTrustBundle,
201-
}
153+
address, port, err := clusterConfig.Nutanix.PrismCentralEndpoint.ParseURL()
154+
if err != nil {
155+
return "", err
156+
}
157+
templateInput := input{
158+
PrismCentralHost: address,
159+
PrismCentralPort: port,
160+
PrismCentralInsecure: clusterConfig.Nutanix.PrismCentralEndpoint.Insecure,
161+
PrismCentralAdditionalTrustBundle: clusterConfig.Nutanix.PrismCentralEndpoint.AdditionalTrustBundle,
162+
}
202163

203-
var b bytes.Buffer
204-
err = helmValuesTemplate.Execute(&b, templateInput)
205-
if err != nil {
206-
return "", fmt.Errorf("failed setting PrismCentral configuration in template: %w", err)
207-
}
164+
var b bytes.Buffer
165+
err = helmValuesTemplate.Execute(&b, templateInput)
166+
if err != nil {
167+
return "", fmt.Errorf("failed setting PrismCentral configuration in template: %w", err)
168+
}
208169

209-
return b.String(), nil
170+
return b.String(), nil
171+
}
210172
}

‎pkg/handlers/generic/lifecycle/ccm/nutanix/handler_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ func Test_templateValues(t *testing.T) {
123123
tt := tests[idx]
124124
t.Run(tt.name, func(t *testing.T) {
125125
t.Parallel()
126-
out, err := templateValues(tt.clusterConfig, tt.in)
126+
out, err := templateValuesFunc(tt.clusterConfig)(nil, tt.in)
127127
require.NoError(t, err)
128128
assert.Equal(t, tt.expected, out)
129129
})

0 commit comments

Comments
 (0)