Skip to content

SKS-4619: Use secret to create tower client #213

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

Open
haijianyang wants to merge 1 commit into
base: master
Choose a base branch
from
Open

SKS-4619: Use secret to create tower client #213

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
16 changes: 16 additions & 0 deletions api/v1beta1/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,22 @@ func (z ElfClusterZoneType) ToLower() string {
}

type Tower struct {
TowerClientConfig `json:",inline"`

// SecretRef is the reference to the secret containing the tower information.
SecretRef *corev1.SecretReference `json:"secretRef,omitempty"`
}

func (t *Tower) String() string {
if t.SecretRef != nil {
return fmt.Sprintf("%s/%s", t.SecretRef.Namespace, t.SecretRef.Name)
}

return t.TowerClientConfig.Server
}

// TowerClientConfig is the connection information for the tower server.
type TowerClientConfig struct {
// Server is address of the tower server.
Server string `json:"server,omitempty"`

Expand Down
25 changes: 23 additions & 2 deletions api/v1beta1/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.

16 changes: 15 additions & 1 deletion config/crd/bases/infrastructure.cluster.x-k8s.io_elfclusters.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
annotations:
controller-gen.kubebuilder.io/version: v0.17.2
controller-gen.kubebuilder.io/version: v0.18.0
name: elfclusters.infrastructure.cluster.x-k8s.io
spec:
group: infrastructure.cluster.x-k8s.io
Expand Down Expand Up @@ -97,6 +97,20 @@ spec:
description: Password is the password used to access the tower
server.
type: string
secretRef:
description: SecretRef is the reference to the secret containing
the tower information.
properties:
name:
description: name is unique within a namespace to reference
a secret resource.
type: string
namespace:
description: namespace defines the space within which the
secret name must be unique.
type: string
type: object
x-kubernetes-map-type: atomic
server:
description: Server is address of the tower server.
type: string
Expand Down
2 changes: 1 addition & 1 deletion config/crd/bases/infrastructure.cluster.x-k8s.io_elfmachines.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
annotations:
controller-gen.kubebuilder.io/version: v0.17.2
controller-gen.kubebuilder.io/version: v0.18.0
name: elfmachines.infrastructure.cluster.x-k8s.io
spec:
group: infrastructure.cluster.x-k8s.io
Expand Down
2 changes: 1 addition & 1 deletion config/crd/bases/infrastructure.cluster.x-k8s.io_elfmachinetemplates.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
annotations:
controller-gen.kubebuilder.io/version: v0.17.2
controller-gen.kubebuilder.io/version: v0.18.0
name: elfmachinetemplates.infrastructure.cluster.x-k8s.io
spec:
group: infrastructure.cluster.x-k8s.io
Expand Down
14 changes: 7 additions & 7 deletions controllers/elfcluster_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ func (r *ElfClusterReconciler) Reconcile(ctx goctx.Context, req ctrl.Request) (_
// If ElfCluster is being deleting and ForceDeleteCluster flag is set, skip creating the VMService object,
// because Tower server may be out of service. So we can force delete ElfCluster.
if elfCluster.ObjectMeta.DeletionTimestamp.IsZero() || !elfCluster.HasForceDeleteCluster() {
vmService, err := r.NewVMService(ctx, elfCluster.GetTower(), log)
vmService, err := r.NewVMService(ctx, r.Client, elfCluster.GetTower(), log)
if err != nil {
conditions.MarkFalse(&elfCluster, infrav1.TowerAvailableCondition, infrav1.TowerUnreachableReason, clusterv1.ConditionSeverityError, err.Error())

Expand Down Expand Up @@ -283,25 +283,25 @@ func (r *ElfClusterReconciler) cleanOrphanLabels(ctx goctx.Context, clusterCtx *
log := ctrl.LoggerFrom(ctx)

// Locking ensures that only one coroutine cleans at the same time
if ok := acquireLockForGCTowerLabels(clusterCtx.ElfCluster.Spec.Tower.Server); ok {
defer releaseLockForForGCTowerLabels(clusterCtx.ElfCluster.Spec.Tower.Server)
if ok := acquireLockForGCTowerLabels(clusterCtx.ElfCluster.Spec.Tower.String()); ok {
defer releaseLockForForGCTowerLabels(clusterCtx.ElfCluster.Spec.Tower.String())
} else {
return
}

log.V(1).Info(fmt.Sprintf("Cleaning orphan labels in Tower %s created by CAPE", clusterCtx.ElfCluster.Spec.Tower.Server))
log.V(1).Info(fmt.Sprintf("Cleaning orphan labels in Tower %s created by CAPE", clusterCtx.ElfCluster.Spec.Tower.String()))

keys := []string{towerresources.GetVMLabelClusterName(), towerresources.GetVMLabelVIP(), towerresources.GetVMLabelNamespace()}
labelIDs, err := clusterCtx.VMService.CleanUnusedLabels(keys)
if err != nil {
log.Error(err, "Warning: failed to clean orphan labels in Tower "+clusterCtx.ElfCluster.Spec.Tower.Server)
log.Error(err, "Warning: failed to clean orphan labels in Tower "+clusterCtx.ElfCluster.Spec.Tower.String())

return
}

recordGCTimeForTowerLabels(clusterCtx.ElfCluster.Spec.Tower.Server)
recordGCTimeForTowerLabels(clusterCtx.ElfCluster.Spec.Tower.String())

log.V(1).Info(fmt.Sprintf("Labels of Tower %s are cleaned successfully", clusterCtx.ElfCluster.Spec.Tower.Server), "labelCount", len(labelIDs))
log.V(1).Info(fmt.Sprintf("Labels of Tower %s are cleaned successfully", clusterCtx.ElfCluster.Spec.Tower.String()), "labelCount", len(labelIDs))
}

func (r *ElfClusterReconciler) reconcileNormal(ctx goctx.Context, clusterCtx *context.ClusterContext) (reconcile.Result, error) { //nolint:unparam
Expand Down
13 changes: 7 additions & 6 deletions controllers/elfcluster_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
capiutil "sigs.k8s.io/cluster-api/util"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
ctrlutil "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"sigs.k8s.io/controller-runtime/pkg/event"
ctrllog "sigs.k8s.io/controller-runtime/pkg/log"
Expand Down Expand Up @@ -71,7 +72,7 @@ var _ = Describe("ElfClusterReconciler", func() {
// mock
mockCtrl = gomock.NewController(GinkgoT())
mockVMService = mock_services.NewMockVMService(mockCtrl)
mockNewVMService = func(_ goctx.Context, _ infrav1.Tower, _ logr.Logger) (service.VMService, error) {
mockNewVMService = func(_ goctx.Context, _ client.Client, _ infrav1.Tower, _ logr.Logger) (service.VMService, error) {
return mockVMService, nil
}
})
Expand Down Expand Up @@ -218,7 +219,7 @@ var _ = Describe("ElfClusterReconciler", func() {
})

It("should delete failed when tower is out of service", func() {
mockNewVMService = func(_ goctx.Context, _ infrav1.Tower, _ logr.Logger) (service.VMService, error) {
mockNewVMService = func(_ goctx.Context, _ client.Client, _ infrav1.Tower, _ logr.Logger) (service.VMService, error) {
return mockVMService, errors.New("get vm service failed")
}
ctrlMgrCtx := fake.NewControllerManagerContext(elfCluster, cluster)
Expand All @@ -234,7 +235,7 @@ var _ = Describe("ElfClusterReconciler", func() {
})

It("should force delete when tower is out of service and cluster need to force delete", func() {
mockNewVMService = func(_ goctx.Context, _ infrav1.Tower, _ logr.Logger) (service.VMService, error) {
mockNewVMService = func(_ goctx.Context, _ client.Client, _ infrav1.Tower, _ logr.Logger) (service.VMService, error) {
return mockVMService, errors.New("get vm service failed")
}
elfCluster.Annotations = map[string]string{
Expand Down Expand Up @@ -275,16 +276,16 @@ var _ = Describe("ElfClusterReconciler", func() {
mockVMService.EXPECT().CleanUnusedLabels(keys).Return(nil, unexpectedError)
reconciler := &ElfClusterReconciler{ControllerManagerContext: ctrlMgrCtx, NewVMService: mockNewVMService}
reconciler.cleanOrphanLabels(ctx, clusterCtx)
Expect(logBuffer.String()).To(ContainSubstring("Warning: failed to clean orphan labels in Tower " + elfCluster.Spec.Tower.Server))
Expect(logBuffer.String()).To(ContainSubstring("Warning: failed to clean orphan labels in Tower " + elfCluster.Spec.Tower.String()))

logBuffer.Reset()
mockVMService.EXPECT().CleanUnusedLabels(keys).Return(nil, nil)
reconciler.cleanOrphanLabels(ctx, clusterCtx)
Expect(logBuffer.String()).To(ContainSubstring(fmt.Sprintf("Labels of Tower %s are cleaned successfully", elfCluster.Spec.Tower.Server)))
Expect(logBuffer.String()).To(ContainSubstring(fmt.Sprintf("Labels of Tower %s are cleaned successfully", elfCluster.Spec.Tower.String())))

logBuffer.Reset()
reconciler.cleanOrphanLabels(ctx, clusterCtx)
Expect(logBuffer.String()).NotTo(ContainSubstring(fmt.Sprintf("Cleaning orphan labels in Tower %s created by CAPE", elfCluster.Spec.Tower.Server)))
Expect(logBuffer.String()).NotTo(ContainSubstring(fmt.Sprintf("Cleaning orphan labels in Tower %s created by CAPE", elfCluster.Spec.Tower.String())))
})
})

Expand Down
2 changes: 1 addition & 1 deletion controllers/elfmachine_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ func (r *ElfMachineReconciler) Reconcile(ctx goctx.Context, req ctrl.Request) (r
// If ElfMachine is being deleting and ElfCLuster ForceDeleteCluster flag is set, skip creating the VMService object,
// because Tower server may be out of service. So we can force delete ElfCluster.
if elfMachine.ObjectMeta.DeletionTimestamp.IsZero() || !elfCluster.HasForceDeleteCluster() {
vmService, err := r.NewVMService(ctx, elfCluster.GetTower(), log)
vmService, err := r.NewVMService(ctx, r.Client, elfCluster.GetTower(), log)
if err != nil {
conditions.MarkFalse(&elfMachine, infrav1.TowerAvailableCondition, infrav1.TowerUnreachableReason, clusterv1.ConditionSeverityError, err.Error())

Expand Down
30 changes: 11 additions & 19 deletions controllers/elfmachine_controller_cloudinit.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
goctx "context"
"fmt"
"io"
"strconv"
"strings"

"github.com/pkg/errors"
Expand All @@ -17,6 +16,7 @@ const (
kubeadmAPIVersionV1Beta3 = "kubeadm.k8s.io/v1beta3"
kubeadmAPIVersionV1Beta4 = "kubeadm.k8s.io/v1beta4"
kubeadmProviderIDValue = "elf://{{ ds.meta_data.instance_id }}"
yamlTagString = "!!str"
)

type cloudInitMutationContext struct {
Expand Down Expand Up @@ -107,7 +107,7 @@ func ensureKubeadmConfigInWriteFiles(root *yaml.Node, mutationCtx cloudInitMutat
continue
}

updated, err := ensureKubeadmConfigContent(content, mutationCtx.hostName)
updated, err := ensureKubeadmConfigContent(content)
if err != nil {
return false, err
}
Expand All @@ -119,8 +119,8 @@ func ensureKubeadmConfigInWriteFiles(root *yaml.Node, mutationCtx cloudInitMutat
return changed, nil
}

func ensureKubeadmConfigContent(content *yaml.Node, hostname string) (bool, error) {
if content.Kind != yaml.ScalarNode || (content.Tag != "" && content.Tag != "!!str") {
func ensureKubeadmConfigContent(content *yaml.Node) (bool, error) {
if content.Kind != yaml.ScalarNode || (content.Tag != "" && content.Tag != yamlTagString) {
return false, nil
}

Expand All @@ -130,7 +130,7 @@ func ensureKubeadmConfigContent(content *yaml.Node, hostname string) (bool, erro
return false, nil
}

if !ensureKubeadmNodeRegistrationDocuments(documents, hostname) {
if !ensureKubeadmNodeRegistrationDocuments(documents) {
return false, nil
}

Expand All @@ -139,20 +139,20 @@ func ensureKubeadmConfigContent(content *yaml.Node, hostname string) (bool, erro
return false, errors.Wrap(err, "failed to marshal kubeadm config after ensuring provider-id")
}

content.Tag = "!!str"
content.Tag = yamlTagString
content.Value = marshaled

return true, nil
}

func ensureKubeadmNodeRegistrationDocuments(documents []*yaml.Node, hostname string) bool {
func ensureKubeadmNodeRegistrationDocuments(documents []*yaml.Node) bool {
changed := false
for _, document := range documents {
if !isKubeadmNodeRegistrationDocument(document) {
continue
}

if ensureKubeadmNodeRegistration(document, hostname) {
if ensureKubeadmNodeRegistration(document) {
changed = true
}
}
Expand All @@ -169,7 +169,7 @@ func isKubeadmNodeRegistrationDocument(root *yaml.Node) bool {
}
}

func ensureKubeadmNodeRegistration(root *yaml.Node, hostname string) bool {
func ensureKubeadmNodeRegistration(root *yaml.Node) bool {
changed := false
nodeRegistration, _ := ensureYAMLMappingValue(root, "nodeRegistration")
if ensureKubeletProviderID(root, nodeRegistration) {
Expand Down Expand Up @@ -368,7 +368,7 @@ func upsertYAMLMapString(parent *yaml.Node, key, value string) bool {
return false
}

existing.Tag = "!!str"
existing.Tag = yamlTagString
existing.Value = value

return true
Expand All @@ -392,14 +392,6 @@ func upsertNamedValueSequenceItem(sequence *yaml.Node, name, value string) bool
return true
}

func newBoolYAMLNode(value bool) *yaml.Node {
return &yaml.Node{
Kind: yaml.ScalarNode,
Tag: "!!bool",
Value: strconv.FormatBool(value),
}
}

func newScalarNodes(values []string) []*yaml.Node {
nodes := make([]*yaml.Node, 0, len(values))
for _, value := range values {
Expand All @@ -420,7 +412,7 @@ func newNamedValueMappingNode(name, value string) *yaml.Node {
func newStringYAMLNode(value string) *yaml.Node {
return &yaml.Node{
Kind: yaml.ScalarNode,
Tag: "!!str",
Tag: yamlTagString,
Value: value,
}
}
4 changes: 2 additions & 2 deletions controllers/elfmachine_controller_cloudinit_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ kind: KubeProxyConfiguration
metricsBindAddress: 0.0.0.0:10249
`)

changed, err := ensureKubeadmConfigContent(content, "")
changed, err := ensureKubeadmConfigContent(content)
if err != nil {
t.Fatalf("ensureKubeadmConfigContent() error = %v", err)
}
Expand Down Expand Up @@ -73,7 +73,7 @@ nodeRegistration:
value: "0"
`)

changed, err := ensureKubeadmConfigContent(content, "")
changed, err := ensureKubeadmConfigContent(content)
if err != nil {
t.Fatalf("ensureKubeadmConfigContent() error = %v", err)
}
Expand Down
2 changes: 1 addition & 1 deletion controllers/elfmachine_controller_gpu_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ var _ = Describe("ElfMachineReconciler-GPU", func() {
// mock
mockCtrl = gomock.NewController(GinkgoT())
mockVMService = mock_services.NewMockVMService(mockCtrl)
mockNewVMService = func(_ goctx.Context, _ infrav1.Tower, _ logr.Logger) (service.VMService, error) {
mockNewVMService = func(_ goctx.Context, _ client.Client, _ infrav1.Tower, _ logr.Logger) (service.VMService, error) {
return mockVMService, nil
}
})
Expand Down
2 changes: 1 addition & 1 deletion controllers/elfmachine_controller_resources_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ var _ = Describe("ElfMachineReconciler", func() {
// mock
mockCtrl = gomock.NewController(GinkgoT())
mockVMService = mock_services.NewMockVMService(mockCtrl)
mockNewVMService = func(_ goctx.Context, _ infrav1.Tower, _ logr.Logger) (service.VMService, error) {
mockNewVMService = func(_ goctx.Context, _ client.Client, _ infrav1.Tower, _ logr.Logger) (service.VMService, error) {
return mockVMService, nil
}
})
Expand Down
Loading
Loading