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

Allow users to define vm naming logic #369

Merged
merged 5 commits into from
Jan 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions PROJECT
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ resources:
version: v1beta1
webhooks:
conversion: true
defaulting: true
validation: true
webhookVersion: v1
- api:
crdVersion: v1
Expand Down
8 changes: 7 additions & 1 deletion api/v1beta1/vcdmachine_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,13 @@ type VCDMachineSpec struct {
// ExtraOvdcNetworks is the list of extra Ovdc Networks that are mounted to machines.
// VCDClusterSpec.OvdcNetwork is always attached regardless of this field.
// +optional
ExtraOvdcNetworks []string `json:"extraOvdcNetworks"`
ExtraOvdcNetworks []string `json:"extraOvdcNetworks,omitempty"`

// VmNamingTemplate is go template to generate VM names based on Machine and VCDMachine CRs.
// Functions of Sprig library are supported. See https://github.com/Masterminds/sprig
// Immutable field. machine.Name is used as VM name when this field is empty.
// +optional
VmNamingTemplate string `json:"vmNamingTemplate,omitempty"`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Users should be able to create VCDMachine CRs without providing empty values for these fields. Missing omitempty tags lead to errors in CAPI controllers while creating VCDMachine by using VCDMachineTemplate.

}

// VCDMachineStatus defines the observed state of VCDMachine
Expand Down
55 changes: 55 additions & 0 deletions api/v1beta1/vcdmachine_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,14 @@ limitations under the License.
package v1beta1

import (
"fmt"

apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/validation/field"
ctrl "sigs.k8s.io/controller-runtime"
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/webhook"
)

// log is for logging in this package.
Expand All @@ -31,3 +37,52 @@ func (r *VCDMachine) SetupWebhookWithManager(mgr ctrl.Manager) error {
}

// TODO(user): EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN!

//+kubebuilder:webhook:path=/mutate-infrastructure-cluster-x-k8s-io-v1beta1-vcdmachine,mutating=true,failurePolicy=fail,sideEffects=None,groups=infrastructure.cluster.x-k8s.io,resources=vcdmachines,verbs=create;update,versions=v1beta1,name=mvcdmachine.kb.io,admissionReviewVersions=v1

var _ webhook.Defaulter = &VCDMachine{}

// Default implements webhook.Defaulter so a webhook will be registered for the type
func (r *VCDMachine) Default() {
vcdmachinelog.Info("default", "name", r.Name)

// TODO(user): fill in your defaulting logic.
}

// TODO(user): change verbs to "verbs=create;update;delete" if you want to enable deletion validation.
//+kubebuilder:webhook:path=/validate-infrastructure-cluster-x-k8s-io-v1beta1-vcdmachine,mutating=false,failurePolicy=fail,sideEffects=None,groups=infrastructure.cluster.x-k8s.io,resources=vcdmachines,verbs=create;update,versions=v1beta1,name=vvcdmachine.kb.io,admissionReviewVersions=v1

var _ webhook.Validator = &VCDMachine{}

// ValidateCreate implements webhook.Validator so a webhook will be registered for the type
func (r *VCDMachine) ValidateCreate() error {
vcdmachinelog.Info("validate create", "name", r.Name)

// TODO(user): fill in your validation logic upon object creation.
return nil
}

// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
func (r *VCDMachine) ValidateUpdate(oldRaw runtime.Object) error {
vcdmachinelog.Info("validate update", "name", r.Name)

old, ok := oldRaw.(*VCDMachine)
if !ok {
return apierrors.NewBadRequest(fmt.Sprintf("expected an VCDMachine but got a %T", oldRaw))
}

// the relation between CR->VM_NAME have to be consistent therefore VmNamingTemplate is immutable
if r.Spec.VmNamingTemplate != old.Spec.VmNamingTemplate {
return field.Forbidden(field.NewPath("spec.vmNamingTemplate"), "cannot be modified")
}

return nil
}

// ValidateDelete implements webhook.Validator so a webhook will be registered for the type
func (r *VCDMachine) ValidateDelete() error {
vcdmachinelog.Info("validate delete", "name", r.Name)

// TODO(user): fill in your validation logic upon object deletion.
return nil
}
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,12 @@ spec:
description: TemplatePath is the path of the template OVA that is
to be used
type: string
vmNamingTemplate:
description: VmNamingTemplate is go template to generate VM names
based on Machine and VCDMachine CRs. Functions of Sprig library
are supported. See https://github.com/Masterminds/sprig Immutable
field. machine.Name is used as VM name when this field is empty.
type: string
type: object
status:
description: VCDMachineStatus defines the observed state of VCDMachine
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,13 @@ spec:
description: TemplatePath is the path of the template OVA
that is to be used
type: string
vmNamingTemplate:
description: VmNamingTemplate is go template to generate VM
names based on Machine and VCDMachine CRs. Functions of
Sprig library are supported. See https://github.com/Masterminds/sprig
Immutable field. machine.Name is used as VM name when this
field is empty.
type: string
type: object
required:
- spec
Expand Down
40 changes: 40 additions & 0 deletions config/webhook/manifests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,26 @@ webhooks:
resources:
- vcdclusters
sideEffects: None
- admissionReviewVersions:
- v1
clientConfig:
service:
name: webhook-service
namespace: system
path: /mutate-infrastructure-cluster-x-k8s-io-v1beta1-vcdmachine
failurePolicy: Fail
name: mvcdmachine.kb.io
rules:
- apiGroups:
- infrastructure.cluster.x-k8s.io
apiVersions:
- v1beta1
operations:
- CREATE
- UPDATE
resources:
- vcdmachines
sideEffects: None

---
apiVersion: admissionregistration.k8s.io/v1
Expand Down Expand Up @@ -59,3 +79,23 @@ webhooks:
resources:
- vcdclusters
sideEffects: None
- admissionReviewVersions:
- v1
clientConfig:
service:
name: webhook-service
namespace: system
path: /validate-infrastructure-cluster-x-k8s-io-v1beta1-vcdmachine
failurePolicy: Fail
name: vvcdmachine.kb.io
rules:
- apiGroups:
- infrastructure.cluster.x-k8s.io
apiVersions:
- v1beta1
operations:
- CREATE
- UPDATE
resources:
- vcdmachines
sideEffects: None
49 changes: 44 additions & 5 deletions controllers/vcdmachine_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ import (
"text/template"
"time"

"github.com/Masterminds/sprig/v3"
"github.com/go-logr/logr"

"github.com/pkg/errors"
cpiutil "github.com/vmware/cloud-provider-for-cloud-director/pkg/util"
"github.com/vmware/cloud-provider-for-cloud-director/pkg/vcdsdk"
Expand Down Expand Up @@ -408,6 +411,11 @@ func (r *VCDMachineReconciler) reconcileNormal(ctx context.Context, cluster *clu
}
// The vApp should have already been created, so this is more of a Get of the vApp
vAppName := cluster.Name
vmName, err := getVMName(machine, vcdMachine, log)
if err != nil {
return ctrl.Result{}, err
}

vApp, err := vdcManager.Vdc.GetVAppByName(vAppName, true)
if err != nil {
updatedErr := capvcdRdeManager.AddToErrorSet(ctx, capisdk.VCDClusterVappCreationError, "", machine.Name, fmt.Sprintf("%v", err))
Expand Down Expand Up @@ -456,7 +464,8 @@ func (r *VCDMachineReconciler) reconcileNormal(ctx context.Context, cluster *clu
cloudInitInput.HTTPProxy = vcdCluster.Spec.ProxyConfigSpec.HTTPProxy
cloudInitInput.HTTPSProxy = vcdCluster.Spec.ProxyConfigSpec.HTTPSProxy
cloudInitInput.NoProxy = vcdCluster.Spec.ProxyConfigSpec.NoProxy
cloudInitInput.MachineName = machine.Name
cloudInitInput.MachineName = vmName

// TODO: After tenants has access to siteId, populate siteId to cloudInitInput as opposed to the site
cloudInitInput.VcdHostFormatted = strings.ReplaceAll(vcdCluster.Spec.Site, "/", "\\/")
cloudInitInput.NvidiaGPU = vcdMachine.Spec.EnableNvidiaGPU
Expand Down Expand Up @@ -490,7 +499,7 @@ func (r *VCDMachineReconciler) reconcileNormal(ctx context.Context, cluster *clu
}

vmExists := true
vm, err := vApp.GetVMByName(machine.Name, true)
vm, err := vApp.GetVMByName(vmName, true)
if err != nil && err != govcd.ErrorEntityNotFound {
updatedErr := capvcdRdeManager.AddToErrorSet(ctx, capisdk.VCDMachineCreationError, "", machine.Name, fmt.Sprintf("%v", err))
if updatedErr != nil {
Expand All @@ -505,7 +514,7 @@ func (r *VCDMachineReconciler) reconcileNormal(ctx context.Context, cluster *clu
log.Info("Adding infra VM for the machine")

// vcda-4391 fixed
err = vdcManager.AddNewVM(machine.Name, vcdCluster.Name, 1,
err = vdcManager.AddNewVM(vmName, vcdCluster.Name, 1,
vcdMachine.Spec.Catalog, vcdMachine.Spec.Template, vcdMachine.Spec.PlacementPolicy,
vcdMachine.Spec.SizingPolicy, vcdMachine.Spec.StorageProfile, "", false)
if err != nil {
Expand All @@ -516,7 +525,7 @@ func (r *VCDMachineReconciler) reconcileNormal(ctx context.Context, cluster *clu
return ctrl.Result{}, errors.Wrapf(err, "Error provisioning infrastructure for the machine; unable to create VM [%s] in vApp [%s]",
machine.Name, vApp.VApp.Name)
}
vm, err = vApp.GetVMByName(machine.Name, true)
vm, err = vApp.GetVMByName(vmName, true)
if err != nil {
err1 := capvcdRdeManager.AddToErrorSet(ctx, capisdk.VCDMachineCreationError, "", machine.Name, fmt.Sprintf("%v", err))
if err1 != nil {
Expand Down Expand Up @@ -867,6 +876,32 @@ func (r *VCDMachineReconciler) reconcileNormal(ctx context.Context, cluster *clu
return ctrl.Result{}, nil
}

func getVMName(machine *clusterv1.Machine, vcdMachine *infrav1.VCDMachine, log logr.Logger) (string, error) {
if vcdMachine.Spec.VmNamingTemplate == "" {
return machine.Name, nil
}

vmNameTemplate, err := template.New("vmname").
Funcs(sprig.TxtFuncMap()).
Parse(vcdMachine.Spec.VmNamingTemplate)
if err != nil {
log.Error(err, "Error while parsing VmNamingTemplate of VCDMachine")
return "", errors.Wrapf(err, "Error while parsing VmNamingTemplate of VCDMachine")
}

buf := new(bytes.Buffer)
err = vmNameTemplate.Execute(buf, map[string]interface{}{
"machine": machine,
"vcdMachine": vcdMachine,
})
if err != nil {
log.Error(err, "Error while generating VM Name by using VmNamingTemplate of VCDMachine")
return "", errors.Wrapf(err, "Error while generating VM Name by using VmNamingTemplate of VCDMachine")
}

return buf.String(), nil
}

// getPrimaryNetwork returns the primary network based on vm.NetworkConnectionSection.PrimaryNetworkConnectionIndex
// It is not possible to assume vm.NetworkConnectionSection.NetworkConnection[0] is the primary network when there are
// multiple networks attached to the VM.
Expand Down Expand Up @@ -1177,7 +1212,11 @@ func (r *VCDMachineReconciler) reconcileDelete(ctx context.Context, cluster *clu
log.Error(err, "failed to remove VCDMachineError from RDE", "rdeID", vcdCluster.Status.InfraId)
}
// delete the vm
vm, err := vApp.GetVMByName(machine.Name, true)
vmName, err := getVMName(machine, vcdMachine, log)
if err != nil {
return ctrl.Result{}, err
}
vm, err := vApp.GetVMByName(vmName, true)
if err != nil {
if err == govcd.ErrorEntityNotFound {
log.Error(err, "Error while deleting the machine; VM not found")
Expand Down
10 changes: 9 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ module github.com/vmware/cluster-api-provider-cloud-director
go 1.17

require (
github.com/Masterminds/sprig/v3 v3.2.2
github.com/antihax/optional v1.0.0
github.com/blang/semver v3.5.1+incompatible
github.com/go-logr/logr v1.2.2
github.com/google/uuid v1.3.0
github.com/onsi/ginkgo v1.16.5
github.com/onsi/gomega v1.19.0
Expand All @@ -29,6 +31,8 @@ require (
github.com/Azure/go-autorest/autorest/date v0.3.0 // indirect
github.com/Azure/go-autorest/logger v0.2.1 // indirect
github.com/Azure/go-autorest/tracing v0.6.0 // indirect
github.com/Masterminds/goutils v1.1.1 // indirect
github.com/Masterminds/semver/v3 v3.1.1 // indirect
github.com/PuerkitoBio/purell v1.1.1 // indirect
github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578 // indirect
github.com/apparentlymart/go-cidr v1.1.0 // indirect
Expand All @@ -43,7 +47,6 @@ require (
github.com/evanphx/json-patch v4.12.0+incompatible // indirect
github.com/evanphx/json-patch/v5 v5.6.0 // indirect
github.com/fsnotify/fsnotify v1.5.1 // indirect
github.com/go-logr/logr v1.2.2 // indirect
github.com/go-logr/zapr v1.2.0 // indirect
github.com/go-openapi/errors v0.20.2 // indirect
github.com/go-openapi/jsonpointer v0.19.5 // indirect
Expand All @@ -58,14 +61,17 @@ require (
github.com/google/go-cmp v0.5.6 // indirect
github.com/google/gofuzz v1.2.0 // indirect
github.com/hashicorp/go-version v1.3.0 // indirect
github.com/huandu/xstrings v1.3.2 // indirect
github.com/imdario/mergo v0.3.12 // indirect
github.com/josharian/intern v1.0.0 // indirect
github.com/json-iterator/go v1.1.12 // indirect
github.com/kr/pretty v0.2.1 // indirect
github.com/kr/text v0.2.0 // indirect
github.com/mailru/easyjson v0.7.6 // indirect
github.com/matttproud/golang_protobuf_extensions v1.0.2-0.20181231171920-c182affec369 // indirect
github.com/mitchellh/copystructure v1.2.0 // indirect
github.com/mitchellh/mapstructure v1.4.3 // indirect
github.com/mitchellh/reflectwalk v1.0.2 // indirect
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
github.com/modern-go/reflect2 v1.0.2 // indirect
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
Expand All @@ -76,6 +82,8 @@ require (
github.com/prometheus/client_model v0.2.0 // indirect
github.com/prometheus/common v0.32.1 // indirect
github.com/prometheus/procfs v0.7.3 // indirect
github.com/shopspring/decimal v1.2.0 // indirect
github.com/spf13/cast v1.4.1 // indirect
github.com/spf13/pflag v1.0.5 // indirect
github.com/stretchr/testify v1.7.2 // indirect
go.uber.org/atomic v1.7.0 // indirect
Expand Down
Loading