Skip to content

Commit

Permalink
User klog.Background() instead of textlogger (kubernetes-sigs#1778)
Browse files Browse the repository at this point in the history
  • Loading branch information
Karthik-K-N authored and Amulyam24 committed May 16, 2024
1 parent dcf1882 commit e970b63
Show file tree
Hide file tree
Showing 16 changed files with 93 additions and 79 deletions.
4 changes: 2 additions & 2 deletions cloud/scope/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import (
"github.com/IBM/go-sdk-core/v5/core"
"github.com/IBM/vpc-go-sdk/vpcv1"

"k8s.io/klog/v2/textlogger"
"k8s.io/klog/v2"

"sigs.k8s.io/controller-runtime/pkg/client"

Expand Down Expand Up @@ -74,7 +74,7 @@ func NewClusterScope(params ClusterScopeParams) (*ClusterScope, error) {
}

if params.Logger == (logr.Logger{}) {
params.Logger = textlogger.NewLogger(textlogger.NewConfig())
params.Logger = klog.Background()
}

helper, err := patch.NewHelper(params.IBMVPCCluster, params.Client)
Expand Down
4 changes: 2 additions & 2 deletions cloud/scope/cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import (
"go.uber.org/mock/gomock"

"k8s.io/client-go/kubernetes/scheme"
"k8s.io/klog/v2/textlogger"
"k8s.io/klog/v2"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"

Expand All @@ -47,7 +47,7 @@ func setupClusterScope(clusterName string, mockvpc *mock.MockVpc) *ClusterScope
client := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects(initObjects...).Build()
return &ClusterScope{
Client: client,
Logger: textlogger.NewLogger(textlogger.NewConfig()),
Logger: klog.Background(),
IBMVPCClient: mockvpc,
Cluster: cluster,
IBMVPCCluster: vpcCluster,
Expand Down
4 changes: 2 additions & 2 deletions cloud/scope/machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import (

corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/klog/v2/textlogger"
"k8s.io/klog/v2"
"k8s.io/utils/ptr"

"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -80,7 +80,7 @@ func NewMachineScope(params MachineScopeParams) (*MachineScope, error) {
}

if params.Logger == (logr.Logger{}) {
params.Logger = textlogger.NewLogger(textlogger.NewConfig())
params.Logger = klog.Background()
}

helper, err := patch.NewHelper(params.IBMVPCMachine, params.Client)
Expand Down
4 changes: 2 additions & 2 deletions cloud/scope/machine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import (
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes/scheme"
"k8s.io/klog/v2/textlogger"
"k8s.io/klog/v2"
capiv1beta1 "sigs.k8s.io/cluster-api/api/v1beta1"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
Expand Down Expand Up @@ -66,7 +66,7 @@ func setupMachineScope(clusterName string, machineName string, mockvpc *mock.Moc
client := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects(initObjects...).Build()
return &MachineScope{
Client: client,
Logger: textlogger.NewLogger(textlogger.NewConfig()),
Logger: klog.Background(),
IBMVPCClient: mockvpc,
Cluster: cluster,
Machine: machine,
Expand Down
56 changes: 36 additions & 20 deletions cloud/scope/powervs_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import (
"github.com/IBM/platform-services-go-sdk/resourcemanagerv2"
"github.com/IBM/vpc-go-sdk/vpcv1"

"k8s.io/klog/v2/textlogger"
"k8s.io/klog/v2"
"k8s.io/utils/ptr"

"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -117,7 +117,7 @@ func NewPowerVSClusterScope(params PowerVSClusterScopeParams) (*PowerVSClusterSc
return nil, err
}
if params.Logger == (logr.Logger{}) {
params.Logger = textlogger.NewLogger(textlogger.NewConfig())
params.Logger = klog.Background()
}

helper, err := patch.NewHelper(params.IBMPowerVSCluster, params.Client)
Expand Down Expand Up @@ -1841,6 +1841,8 @@ func (s *PowerVSClusterScope) GetServiceName(resourceType infrav1beta2.ResourceT

// DeleteLoadBalancer deletes loadBalancer.
func (s *PowerVSClusterScope) DeleteLoadBalancer() (bool, error) {
errs := []error{}
requeue := false
for _, lb := range s.IBMPowerVSCluster.Status.LoadBalancers {
if lb.ID == nil || lb.ControllerCreated == nil || !*lb.ControllerCreated {
continue
Expand All @@ -1853,9 +1855,10 @@ func (s *PowerVSClusterScope) DeleteLoadBalancer() (bool, error) {
if err != nil {
if strings.Contains(err.Error(), string(VPCLoadBalancerNotFound)) {
s.Info("VPC load balancer successfully deleted")
return false, nil
continue
}
return false, fmt.Errorf("error fetching the load balancer: %w", err)
errs = append(errs, fmt.Errorf("error fetching the load balancer: %w", err))
continue
}

if lb != nil && lb.ProvisioningStatus != nil && *lb.ProvisioningStatus == string(infrav1beta2.VPCLoadBalancerStateDeletePending) {
Expand All @@ -1866,16 +1869,21 @@ func (s *PowerVSClusterScope) DeleteLoadBalancer() (bool, error) {
if _, err = s.IBMVPCClient.DeleteLoadBalancer(&vpcv1.DeleteLoadBalancerOptions{
ID: lb.ID,
}); err != nil {
s.Error(err, "error deleting the load balancer")
return false, err
errs = append(errs, fmt.Errorf("error deleting the load balancer: %w", err))
continue
}
return true, nil
requeue = true
}
return false, nil
if len(errs) > 0 {
return false, kerrors.NewAggregate(errs)
}
return requeue, nil
}

// DeleteVPCSubnet deletes VPC subnet.
func (s *PowerVSClusterScope) DeleteVPCSubnet() (bool, error) {
errs := []error{}
requeue := false
for _, subnet := range s.IBMPowerVSCluster.Status.VPCSubnet {
if subnet.ID == nil || subnet.ControllerCreated == nil || !*subnet.ControllerCreated {
continue
Expand All @@ -1888,9 +1896,10 @@ func (s *PowerVSClusterScope) DeleteVPCSubnet() (bool, error) {
if err != nil {
if strings.Contains(err.Error(), string(VPCSubnetNotFound)) {
s.Info("VPC subnet successfully deleted")
return false, nil
continue
}
return false, fmt.Errorf("error fetching the subnet: %w", err)
errs = append(errs, fmt.Errorf("error fetching the subnet: %w", err))
continue
}

if net != nil && net.Status != nil && *net.Status == string(infrav1beta2.VPCSubnetStateDeleting) {
Expand All @@ -1900,11 +1909,15 @@ func (s *PowerVSClusterScope) DeleteVPCSubnet() (bool, error) {
if _, err = s.IBMVPCClient.DeleteSubnet(&vpcv1.DeleteSubnetOptions{
ID: net.ID,
}); err != nil {
return false, fmt.Errorf("error deleting VPC subnet: %w", err)
errs = append(errs, fmt.Errorf("error deleting VPC subnet: %w", err))
continue
}
return true, nil
requeue = true
}
return false, nil
if len(errs) > 0 {
return false, kerrors.NewAggregate(errs)
}
return requeue, nil
}

// DeleteVPC deletes VPC.
Expand Down Expand Up @@ -2057,14 +2070,17 @@ func (s *PowerVSClusterScope) DeleteServiceInstance() (bool, error) {
return false, nil
}

servers, err := s.IBMPowerVSClient.GetAllDHCPServers()
if err != nil {
return false, fmt.Errorf("error fetching networks in the PowerVS service instance: %w", err)
}
// If PowerVS service instance is in failed state, proceed with deletion instead of checking for existing network resources.
if serviceInstance != nil && *serviceInstance.State != string(infrav1beta2.ServiceInstanceStateFailed) {
servers, err := s.IBMPowerVSClient.GetAllDHCPServers()
if err != nil {
return false, fmt.Errorf("error fetching networks in the PowerVS service instance: %w", err)
}

if len(servers) > 0 {
s.Info("Wait for DHCP server to be deleted before deleting PowerVS service instance")
return true, nil
if len(servers) > 0 {
s.Info("Wait for DHCP server to be deleted before deleting PowerVS service instance")
return true, nil
}
}

if _, err = s.ResourceClient.DeleteResourceInstance(&resourcecontrollerv2.DeleteResourceInstanceOptions{
Expand Down
4 changes: 2 additions & 2 deletions cloud/scope/powervs_image.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import (
"github.com/IBM/go-sdk-core/v5/core"
"github.com/IBM/platform-services-go-sdk/resourcecontrollerv2"

"k8s.io/klog/v2/textlogger"
"k8s.io/klog/v2"

"sigs.k8s.io/controller-runtime/pkg/client"

Expand Down Expand Up @@ -80,7 +80,7 @@ func NewPowerVSImageScope(params PowerVSImageScopeParams) (scope *PowerVSImageSc
scope.IBMPowerVSImage = params.IBMPowerVSImage

if params.Logger == (logr.Logger{}) {
params.Logger = textlogger.NewLogger(textlogger.NewConfig())
params.Logger = klog.Background()
}
scope.Logger = params.Logger

Expand Down
4 changes: 2 additions & 2 deletions cloud/scope/powervs_image_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import (

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes/scheme"
"k8s.io/klog/v2/textlogger"
"k8s.io/klog/v2"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"

Expand Down Expand Up @@ -63,7 +63,7 @@ func setupPowerVSImageScope(imageName string, mockpowervs *mock.MockPowerVS) *Po
client := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects(initObjects...).Build()
return &PowerVSImageScope{
Client: client,
Logger: textlogger.NewLogger(textlogger.NewConfig()),
Logger: klog.Background(),
IBMPowerVSClient: mockpowervs,
IBMPowerVSImage: powervsImage,
}
Expand Down
4 changes: 2 additions & 2 deletions cloud/scope/powervs_machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ import (
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/client-go/tools/cache"
"k8s.io/klog/v2/textlogger"
"k8s.io/klog/v2"
"k8s.io/utils/ptr"

"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -133,7 +133,7 @@ func NewPowerVSMachineScope(params PowerVSMachineScopeParams) (scope *PowerVSMac
scope.IBMPowerVSImage = params.IBMPowerVSImage

if params.Logger == (logr.Logger{}) {
params.Logger = textlogger.NewLogger(textlogger.NewConfig())
params.Logger = klog.Background()
}
if params.Logger.V(DEBUGLEVEL).Enabled() {
core.SetLoggingLevel(core.LevelDebug)
Expand Down
4 changes: 2 additions & 2 deletions cloud/scope/powervs_machine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import (
"k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/client-go/kubernetes/scheme"
"k8s.io/client-go/tools/cache"
"k8s.io/klog/v2/textlogger"
"k8s.io/klog/v2"
"k8s.io/utils/ptr"
capiv1beta1 "sigs.k8s.io/cluster-api/api/v1beta1"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -89,7 +89,7 @@ func setupPowerVSMachineScope(clusterName string, machineName string, imageID *s
client := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects(initObjects...).Build()
return &PowerVSMachineScope{
Client: client,
Logger: textlogger.NewLogger(textlogger.NewConfig()),
Logger: klog.Background(),
IBMPowerVSClient: mockpowervs,
Cluster: cluster,
Machine: machine,
Expand Down
6 changes: 3 additions & 3 deletions controllers/ibmpowervscluster_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes/scheme"
"k8s.io/klog/v2/textlogger"
"k8s.io/klog/v2"
"k8s.io/utils/ptr"
capiv1beta1 "sigs.k8s.io/cluster-api/api/v1beta1"
"sigs.k8s.io/cluster-api/util"
Expand Down Expand Up @@ -178,7 +178,7 @@ func TestIBMPowerVSClusterReconciler_delete(t *testing.T) {
t.Run("Should reconcile successfully if no descendants are found", func(t *testing.T) {
g := NewWithT(t)
clusterScope = &scope.PowerVSClusterScope{
Logger: textlogger.NewLogger(textlogger.NewConfig()),
Logger: klog.Background(),
IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{
TypeMeta: metav1.TypeMeta{
Kind: "IBMPowerVSCluster",
Expand All @@ -200,7 +200,7 @@ func TestIBMPowerVSClusterReconciler_delete(t *testing.T) {
t.Run("Should reconcile with requeue by deleting the cluster descendants", func(t *testing.T) {
g := NewWithT(t)
clusterScope = &scope.PowerVSClusterScope{
Logger: textlogger.NewLogger(textlogger.NewConfig()),
Logger: klog.Background(),
IBMPowerVSCluster: &infrav1beta2.IBMPowerVSCluster{
TypeMeta: metav1.TypeMeta{
Kind: "IBMPowerVSCluster",
Expand Down
8 changes: 4 additions & 4 deletions controllers/ibmpowervsimage_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import (
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/tools/record"
"k8s.io/klog/v2/textlogger"
"k8s.io/klog/v2"
"k8s.io/utils/ptr"
capiv1beta1 "sigs.k8s.io/cluster-api/api/v1beta1"
"sigs.k8s.io/cluster-api/util"
Expand Down Expand Up @@ -151,7 +151,7 @@ func TestIBMPowerVSImageReconciler_reconcile(t *testing.T) {
Name: "capi-powervs-cluster"},
}
imageScope := &scope.PowerVSImageScope{
Logger: textlogger.NewLogger(textlogger.NewConfig()),
Logger: klog.Background(),
IBMPowerVSImage: &infrav1beta2.IBMPowerVSImage{
ObjectMeta: metav1.ObjectMeta{
Name: "capi-image",
Expand Down Expand Up @@ -204,7 +204,7 @@ func TestIBMPowerVSImageReconciler_reconcile(t *testing.T) {

mockclient := fake.NewClientBuilder().WithObjects([]client.Object{powervsCluster, powervsImage}...).Build()
imageScope := &scope.PowerVSImageScope{
Logger: textlogger.NewLogger(textlogger.NewConfig()),
Logger: klog.Background(),
Client: mockclient,
IBMPowerVSImage: powervsImage,
IBMPowerVSClient: mockpowervs,
Expand Down Expand Up @@ -337,7 +337,7 @@ func TestIBMPowerVSImageReconciler_delete(t *testing.T) {
Recorder: recorder,
}
imageScope = &scope.PowerVSImageScope{
Logger: textlogger.NewLogger(textlogger.NewConfig()),
Logger: klog.Background(),
IBMPowerVSImage: &infrav1beta2.IBMPowerVSImage{},
IBMPowerVSClient: mockpowervs,
}
Expand Down
Loading

0 comments on commit e970b63

Please sign in to comment.