Skip to content

Commit

Permalink
CFE-684: Add user defined tags to the created gcp resources
Browse files Browse the repository at this point in the history
  • Loading branch information
bharath-b-rh committed Feb 7, 2024
1 parent 40cbe72 commit 002bb45
Show file tree
Hide file tree
Showing 15 changed files with 15,688 additions and 22 deletions.
2 changes: 2 additions & 0 deletions cmd/manager/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"github.com/openshift/machine-api-provider-gcp/pkg/cloud/gcp/actuators/machine"
machinesetcontroller "github.com/openshift/machine-api-provider-gcp/pkg/cloud/gcp/actuators/machineset"
computeservice "github.com/openshift/machine-api-provider-gcp/pkg/cloud/gcp/actuators/services/compute"
tagservice "github.com/openshift/machine-api-provider-gcp/pkg/cloud/gcp/actuators/services/tags"
"github.com/openshift/machine-api-provider-gcp/pkg/version"
corev1 "k8s.io/api/core/v1"
"k8s.io/client-go/kubernetes"
Expand Down Expand Up @@ -156,6 +157,7 @@ func main() {
CoreClient: mgr.GetClient(),
EventRecorder: mgr.GetEventRecorderFor("gcpcontroller"),
ComputeClientBuilder: computeservice.NewComputeService,
TagsClientBuilder: tagservice.NewTagService,
FeatureGates: featureGates,
})

Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ toolchain go1.21.0
require (
github.com/blang/semver v3.5.1+incompatible
github.com/go-logr/logr v1.4.1
github.com/googleapis/gax-go/v2 v2.11.0
github.com/onsi/ginkgo/v2 v2.15.0
github.com/onsi/gomega v1.31.1
github.com/openshift/api v0.0.0-20240124164020-e2ce40831f2e
Expand Down Expand Up @@ -62,7 +63,6 @@ require (
github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 // indirect
github.com/google/uuid v1.4.0 // indirect
github.com/googleapis/enterprise-certificate-proxy v0.2.3 // indirect
github.com/googleapis/gax-go/v2 v2.11.0 // indirect
github.com/gorilla/websocket v1.5.0 // indirect
github.com/gregjones/httpcache v0.0.0-20180305231024-9cad4c3443a7 // indirect
github.com/imdario/mergo v0.3.13 // indirect
Expand Down
8 changes: 8 additions & 0 deletions pkg/cloud/gcp/actuators/machine/actuator.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
machinev1 "github.com/openshift/api/machine/v1beta1"
"github.com/openshift/library-go/pkg/operator/configobserver/featuregates"
computeservice "github.com/openshift/machine-api-provider-gcp/pkg/cloud/gcp/actuators/services/compute"
tagservice "github.com/openshift/machine-api-provider-gcp/pkg/cloud/gcp/actuators/services/tags"
corev1 "k8s.io/api/core/v1"
"k8s.io/client-go/tools/record"
"k8s.io/klog/v2"
Expand All @@ -31,6 +32,7 @@ type Actuator struct {
coreClient controllerclient.Client
eventRecorder record.EventRecorder
computeClientBuilder computeservice.BuilderFuncType
tagsClientBuilder tagservice.BuilderFuncType
featureGates featuregates.FeatureGate
}

Expand All @@ -39,6 +41,7 @@ type ActuatorParams struct {
CoreClient controllerclient.Client
EventRecorder record.EventRecorder
ComputeClientBuilder computeservice.BuilderFuncType
TagsClientBuilder tagservice.BuilderFuncType
FeatureGates featuregates.FeatureGate
}

Expand All @@ -48,6 +51,7 @@ func NewActuator(params ActuatorParams) *Actuator {
coreClient: params.CoreClient,
eventRecorder: params.EventRecorder,
computeClientBuilder: params.ComputeClientBuilder,
tagsClientBuilder: params.TagsClientBuilder,
featureGates: params.FeatureGates,
}
}
Expand All @@ -70,6 +74,7 @@ func (a *Actuator) Create(ctx context.Context, machine *machinev1.Machine) error
coreClient: a.coreClient,
machine: machine,
computeClientBuilder: a.computeClientBuilder,
tagsClientBuilder: a.tagsClientBuilder,
featureGates: a.featureGates,
})
if err != nil {
Expand All @@ -93,6 +98,7 @@ func (a *Actuator) Exists(ctx context.Context, machine *machinev1.Machine) (bool
coreClient: a.coreClient,
machine: machine,
computeClientBuilder: a.computeClientBuilder,
tagsClientBuilder: a.tagsClientBuilder,
featureGates: a.featureGates,
})
if err != nil {
Expand Down Expand Up @@ -133,6 +139,7 @@ func (a *Actuator) Update(ctx context.Context, machine *machinev1.Machine) error
coreClient: a.coreClient,
machine: machine,
computeClientBuilder: a.computeClientBuilder,
tagsClientBuilder: a.tagsClientBuilder,
featureGates: a.featureGates,
})
if err != nil {
Expand Down Expand Up @@ -169,6 +176,7 @@ func (a *Actuator) Delete(ctx context.Context, machine *machinev1.Machine) error
coreClient: a.coreClient,
machine: machine,
computeClientBuilder: a.computeClientBuilder,
tagsClientBuilder: a.tagsClientBuilder,
featureGates: a.featureGates,
})
if err != nil {
Expand Down
8 changes: 8 additions & 0 deletions pkg/cloud/gcp/actuators/machine/actuator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,11 @@ import (
"sigs.k8s.io/controller-runtime/pkg/metrics/server"

. "github.com/onsi/gomega"
configv1 "github.com/openshift/api/config/v1"
machinev1 "github.com/openshift/api/machine/v1beta1"
"github.com/openshift/library-go/pkg/operator/configobserver/featuregates"
computeservice "github.com/openshift/machine-api-provider-gcp/pkg/cloud/gcp/actuators/services/compute"
tagservice "github.com/openshift/machine-api-provider-gcp/pkg/cloud/gcp/actuators/services/tags"
"github.com/openshift/machine-api-provider-gcp/pkg/cloud/gcp/actuators/util"
corev1 "k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -211,6 +214,7 @@ func TestActuatorEvents(t *testing.T) {
name: "Delete machine event succeed",
operation: func(actuator *Actuator, machine *machinev1.Machine) {
actuator.computeClientBuilder = computeservice.MockBuilderFuncTypeNotFound
actuator.tagsClientBuilder = tagservice.NewMockTagServiceBuilder
actuator.Delete(context.Background(), machine)
},
event: "Deleted machine test",
Expand Down Expand Up @@ -278,6 +282,8 @@ func TestActuatorEvents(t *testing.T) {
CoreClient: k8sClient,
EventRecorder: eventRecorder,
ComputeClientBuilder: computeservice.MockBuilderFuncType,
TagsClientBuilder: tagservice.NewMockTagServiceBuilder,
FeatureGates: featuregates.NewFeatureGate(nil, []configv1.FeatureGateName{configv1.FeatureGateGCPLabelsTags}),
}

actuator := NewActuator(params)
Expand Down Expand Up @@ -376,6 +382,8 @@ func TestActuatorExists(t *testing.T) {
params := ActuatorParams{
CoreClient: controllerfake.NewFakeClient(userDataSecret, credentialsSecret),
ComputeClientBuilder: computeservice.MockBuilderFuncType,
TagsClientBuilder: tagservice.NewMockTagServiceBuilder,
FeatureGates: featuregates.NewFeatureGate(nil, []configv1.FeatureGateName{configv1.FeatureGateGCPLabelsTags}),
}

actuator := NewActuator(params)
Expand Down
26 changes: 18 additions & 8 deletions pkg/cloud/gcp/actuators/machine/machine_scope.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/openshift/library-go/pkg/operator/configobserver/featuregates"
machineapierros "github.com/openshift/machine-api-operator/pkg/controller/machine"
computeservice "github.com/openshift/machine-api-provider-gcp/pkg/cloud/gcp/actuators/services/compute"
tagservice "github.com/openshift/machine-api-provider-gcp/pkg/cloud/gcp/actuators/services/tags"
"github.com/openshift/machine-api-provider-gcp/pkg/cloud/gcp/actuators/util"

"k8s.io/apimachinery/pkg/api/equality"
Expand All @@ -24,6 +25,7 @@ type machineScopeParams struct {
coreClient controllerclient.Client
machine *machinev1.Machine
computeClientBuilder computeservice.BuilderFuncType
tagsClientBuilder tagservice.BuilderFuncType
featureGates featuregates.FeatureGate
}

Expand All @@ -50,6 +52,11 @@ type machineScope struct {

// gcpLabelsTagsFeatureEnabled indicates whether FeatureGateGCPLabelsTags is enabled
gcpLabelsTagsFeatureEnabled bool

// tagService is for handling resource manager tags related operations.
tagService tagservice.TagService

featureGates featuregates.FeatureGate
}

// newMachineScope creates a new MachineScope from the supplied parameters.
Expand Down Expand Up @@ -87,10 +94,12 @@ func newMachineScope(params machineScopeParams) (*machineScope, error) {
return nil, machineapierros.InvalidMachineConfiguration("error creating compute service: %v", err)
}

gcpLabelsTagsFeature := false
if params.featureGates != nil {
gcpLabelsTagsFeature = params.featureGates.Enabled(configv1.FeatureGateGCPLabelsTags)
klog.V(1).Infof("status of %v feature is %t", configv1.FeatureGateGCPLabelsTags, gcpLabelsTagsFeature)
var tagService tagservice.TagService
if params.featureGates.Enabled(configv1.FeatureGateGCPLabelsTags) {
tagService, err = params.tagsClientBuilder(params.Context, serviceAccountJSON)
if err != nil {
return nil, machineapierros.InvalidMachineConfiguration("error creating tag service: %v", err)
}
}

return &machineScope{
Expand All @@ -108,10 +117,11 @@ func newMachineScope(params machineScopeParams) (*machineScope, error) {
providerStatus: providerStatus,
// Once set, they can not be changed. Otherwise, status change computation
// might be invalid and result in skipping the status update.
origMachine: params.machine.DeepCopy(),
origProviderStatus: providerStatus.DeepCopy(),
machineToBePatched: controllerclient.MergeFrom(params.machine.DeepCopy()),
gcpLabelsTagsFeatureEnabled: gcpLabelsTagsFeature,
origMachine: params.machine.DeepCopy(),
origProviderStatus: providerStatus.DeepCopy(),
machineToBePatched: controllerclient.MergeFrom(params.machine.DeepCopy()),
featureGates: params.featureGates,
tagService: tagService,
}, nil
}

Expand Down
6 changes: 6 additions & 0 deletions pkg/cloud/gcp/actuators/machine/machine_scope_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ import (
. "github.com/onsi/gomega"
configv1 "github.com/openshift/api/config/v1"
machinev1 "github.com/openshift/api/machine/v1beta1"
"github.com/openshift/library-go/pkg/operator/configobserver/featuregates"
computeservice "github.com/openshift/machine-api-provider-gcp/pkg/cloud/gcp/actuators/services/compute"
tagservice "github.com/openshift/machine-api-provider-gcp/pkg/cloud/gcp/actuators/services/tags"
"github.com/openshift/machine-api-provider-gcp/pkg/cloud/gcp/actuators/util"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -215,6 +217,8 @@ func TestNewMachineScope(t *testing.T) {
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
gs := NewWithT(t)
tc.params.tagsClientBuilder = tagservice.NewMockTagServiceBuilder
tc.params.featureGates = featuregates.NewFeatureGate(nil, []configv1.FeatureGateName{configv1.FeatureGateGCPLabelsTags})
scope, err := newMachineScope(tc.params)

if tc.expectedError != nil {
Expand Down Expand Up @@ -414,6 +418,8 @@ func TestPatchMachine(t *testing.T) {
machine: machine,
Context: ctx,
computeClientBuilder: computeservice.MockBuilderFuncType,
tagsClientBuilder: tagservice.NewMockTagServiceBuilder,
featureGates: featuregates.NewFeatureGate(nil, []configv1.FeatureGateName{configv1.FeatureGateGCPLabelsTags}),
})

gs.Expect(err).ToNot(HaveOccurred())
Expand Down
21 changes: 17 additions & 4 deletions pkg/cloud/gcp/actuators/machine/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"strings"
"time"

configv1 "github.com/openshift/api/config/v1"
machinev1 "github.com/openshift/api/machine/v1beta1"
machinecontroller "github.com/openshift/machine-api-operator/pkg/controller/machine"
"github.com/openshift/machine-api-operator/pkg/metrics"
Expand Down Expand Up @@ -190,6 +191,17 @@ func (r *Reconciler) create() error {
},
}

var userTags map[string]string
if r.featureGates.Enabled(configv1.FeatureGateGCPLabelsTags) {
userTags, err = util.GetResourceManagerTags(r.Context, r.coreClient, r.tagService, r.providerSpec.ResourceManagerTags)
if err != nil {
return machinecontroller.CreateMachine("failed to fetch user-defined tags for %s: %v", r.machine.Name, err)
}
}
instance.Params = &compute.InstanceParams{
ResourceManagerTags: userTags,
}

if automaticRestart, err := restartPolicyToBool(r.providerSpec.RestartPolicy, r.providerSpec.Preemptible); err != nil {
return machinecontroller.InvalidMachineConfiguration("failed to determine restart policy: %v", err)
} else {
Expand Down Expand Up @@ -247,10 +259,11 @@ func (r *Reconciler) create() error {
AutoDelete: disk.AutoDelete,
Boot: disk.Boot,
InitializeParams: &compute.AttachedDiskInitializeParams{
DiskSizeGb: disk.SizeGB,
DiskType: fmt.Sprintf("zones/%s/diskTypes/%s", zone, disk.Type),
Labels: labels,
SourceImage: srcImage,
DiskSizeGb: disk.SizeGB,
DiskType: fmt.Sprintf("zones/%s/diskTypes/%s", zone, disk.Type),
SourceImage: srcImage,
Labels: labels,
ResourceManagerTags: userTags,
},
DiskEncryptionKey: generateDiskEncryptionKey(disk.EncryptionKey, r.projectID),
})
Expand Down

0 comments on commit 002bb45

Please sign in to comment.