diff --git a/cmd/bastion/aws/create.go b/cmd/bastion/aws/create.go index f5abf0965f39..69bc5863769c 100644 --- a/cmd/bastion/aws/create.go +++ b/cmd/bastion/aws/create.go @@ -11,7 +11,6 @@ import ( hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" awsutil "github.com/openshift/hypershift/cmd/infra/aws/util" - "github.com/openshift/hypershift/cmd/log" "github.com/openshift/hypershift/cmd/util" "github.com/aws/aws-sdk-go-v2/aws" @@ -64,7 +63,7 @@ func NewCreateCommand() *cobra.Command { _ = cmd.MarkFlagFilename("ssh-key-file") _ = cmd.MarkFlagFilename("aws-creds") - logger := log.Log + logger := util.NewLogger() cmd.RunE = func(cmd *cobra.Command, args []string) error { if err := opts.Validate(); err != nil { logger.Error(err, "Invalid arguments") diff --git a/cmd/bastion/aws/destroy.go b/cmd/bastion/aws/destroy.go index 2dac765006de..b4232e7effa9 100644 --- a/cmd/bastion/aws/destroy.go +++ b/cmd/bastion/aws/destroy.go @@ -7,7 +7,6 @@ import ( hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" awsutil "github.com/openshift/hypershift/cmd/infra/aws/util" - "github.com/openshift/hypershift/cmd/log" "github.com/openshift/hypershift/cmd/util" "github.com/aws/aws-sdk-go-v2/aws" @@ -49,7 +48,7 @@ func NewDestroyCommand() *cobra.Command { _ = cmd.MarkFlagRequired("aws-creds") _ = cmd.MarkFlagFilename("aws-creds") - logger := log.Log + logger := util.NewLogger() cmd.RunE = func(cmd *cobra.Command, args []string) error { if err := opts.Validate(); err != nil { logger.Error(err, "Invalid arguments") diff --git a/cmd/cluster/agent/destroy.go b/cmd/cluster/agent/destroy.go index 35033a9ef14a..5779fe986f76 100644 --- a/cmd/cluster/agent/destroy.go +++ b/cmd/cluster/agent/destroy.go @@ -6,7 +6,7 @@ import ( "github.com/openshift/hypershift/cmd/cluster/core" "github.com/openshift/hypershift/cmd/cluster/none" - "github.com/openshift/hypershift/cmd/log" + cmdutil "github.com/openshift/hypershift/cmd/util" "github.com/spf13/cobra" ) @@ -24,7 +24,7 @@ func NewDestroyCommand(opts *core.DestroyOptions) *cobra.Command { SilenceUsage: true, } - logger := log.Log + logger := cmdutil.NewLogger() cmd.RunE = func(cmd *cobra.Command, args []string) error { if err := DestroyCluster(cmd.Context(), opts); err != nil { logger.Error(err, "Failed to destroy cluster") diff --git a/cmd/cluster/aws/destroy.go b/cmd/cluster/aws/destroy.go index 707c48420644..dcd005f21a2c 100644 --- a/cmd/cluster/aws/destroy.go +++ b/cmd/cluster/aws/destroy.go @@ -7,7 +7,6 @@ import ( "github.com/openshift/hypershift/cmd/cluster/core" awsinfra "github.com/openshift/hypershift/cmd/infra/aws" awsutil "github.com/openshift/hypershift/cmd/infra/aws/util" - "github.com/openshift/hypershift/cmd/log" "github.com/openshift/hypershift/cmd/util" "k8s.io/apimachinery/pkg/util/errors" @@ -38,7 +37,7 @@ func NewDestroyCommand(opts *core.DestroyOptions) *cobra.Command { opts.AWSPlatform.Credentials.BindFlags(cmd.Flags()) opts.AWSPlatform.VPCOwnerCredentials.BindVPCOwnerFlags(cmd.Flags()) - logger := log.Log + logger := util.NewLogger() cmd.RunE = func(cmd *cobra.Command, args []string) error { err := ValidateCredentialInfo(opts.AWSPlatform.Credentials, opts.CredentialSecretName, opts.Namespace, opts.Kubeconfig) if err != nil { diff --git a/cmd/cluster/azure/destroy.go b/cmd/cluster/azure/destroy.go index baf63d29acb4..c2f4af49e337 100644 --- a/cmd/cluster/azure/destroy.go +++ b/cmd/cluster/azure/destroy.go @@ -11,7 +11,6 @@ import ( hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" "github.com/openshift/hypershift/cmd/cluster/core" azureinfra "github.com/openshift/hypershift/cmd/infra/azure" - "github.com/openshift/hypershift/cmd/log" "github.com/openshift/hypershift/cmd/util" "github.com/openshift/hypershift/support/azureutil" "github.com/openshift/hypershift/support/config" @@ -47,7 +46,7 @@ func NewDestroyCommand(opts *core.DestroyOptions) *cobra.Command { _ = cmd.MarkFlagRequired("azure-creds") _ = cmd.MarkFlagRequired("dns-zone-rg-name") - logger := log.Log + logger := util.NewLogger() cmd.Run = func(cmd *cobra.Command, args []string) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() diff --git a/cmd/cluster/azure/destroy_test.go b/cmd/cluster/azure/destroy_test.go index 1d8f9ef1d52b..79aeb73b4a89 100644 --- a/cmd/cluster/azure/destroy_test.go +++ b/cmd/cluster/azure/destroy_test.go @@ -8,7 +8,7 @@ import ( hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" "github.com/openshift/hypershift/cmd/cluster/core" - "github.com/openshift/hypershift/cmd/log" + cmdutil "github.com/openshift/hypershift/cmd/util" "github.com/openshift/hypershift/support/config" ) @@ -71,7 +71,7 @@ func TestDestroyClusterSetsCloudFromHostedCluster(t *testing.T) { g := NewGomegaWithT(t) opts := &core.DestroyOptions{ ClusterGracePeriod: 10 * time.Minute, - Log: log.Log, + Log: cmdutil.NewLogger(), AzurePlatform: core.AzurePlatformDestroyOptions{ CredentialsFile: "/fake/creds", Location: "eastus", @@ -141,7 +141,7 @@ func TestDestroyClusterSetsGracePeriodFromTopology(t *testing.T) { g := NewGomegaWithT(t) opts := &core.DestroyOptions{ ClusterGracePeriod: test.initialGracePeriod, - Log: log.Log, + Log: cmdutil.NewLogger(), AzurePlatform: core.AzurePlatformDestroyOptions{ CredentialsFile: "/fake/creds", Location: "eastus", diff --git a/cmd/cluster/cluster.go b/cmd/cluster/cluster.go index b9d88c379a93..9b7b68fc273f 100644 --- a/cmd/cluster/cluster.go +++ b/cmd/cluster/cluster.go @@ -12,8 +12,7 @@ import ( "github.com/openshift/hypershift/cmd/cluster/none" "github.com/openshift/hypershift/cmd/cluster/openstack" "github.com/openshift/hypershift/cmd/cluster/powervs" - "github.com/openshift/hypershift/cmd/log" - "github.com/openshift/hypershift/cmd/util" + cmdutil "github.com/openshift/hypershift/cmd/util" "github.com/spf13/cobra" ) @@ -49,7 +48,7 @@ func NewDestroyCommands() *cobra.Command { Namespace: "clusters", Name: "", ClusterGracePeriod: 10 * time.Minute, - Log: log.Log, + Log: cmdutil.NewLogger(), DestroyCloudResources: true, } @@ -58,7 +57,7 @@ func NewDestroyCommands() *cobra.Command { Short: "Destroys a HostedCluster and its associated infrastructure.", SilenceUsage: true, } - cmd.PersistentFlags().StringVar(&opts.Kubeconfig, "kubeconfig", opts.Kubeconfig, util.KubeconfigFlagHelp) + cmd.PersistentFlags().StringVar(&opts.Kubeconfig, "kubeconfig", opts.Kubeconfig, cmdutil.KubeconfigFlagHelp) cmd.PersistentFlags().StringVar(&opts.Namespace, "namespace", opts.Namespace, "A cluster namespace") cmd.PersistentFlags().StringVar(&opts.Name, "name", opts.Name, "A cluster name (required)") cmd.PersistentFlags().DurationVar(&opts.ClusterGracePeriod, "cluster-grace-period", opts.ClusterGracePeriod, "How long to wait for the cluster to be deleted before forcibly destroying its infra") diff --git a/cmd/cluster/core/create.go b/cmd/cluster/core/create.go index 1ab5236f8637..13873bd57b82 100644 --- a/cmd/cluster/core/create.go +++ b/cmd/cluster/core/create.go @@ -13,7 +13,6 @@ import ( hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" "github.com/openshift/hypershift/api/util/ipnet" - "github.com/openshift/hypershift/cmd/log" "github.com/openshift/hypershift/cmd/util" hyperapi "github.com/openshift/hypershift/support/api" "github.com/openshift/hypershift/support/certs" @@ -49,7 +48,7 @@ func DefaultOptions() *RawCreateOptions { ServiceCIDR: []string{globalconfig.DefaultIPv4ServiceCIDR}, ClusterCIDR: []string{globalconfig.DefaultIPv4ClusterCIDR}, MachineCIDR: []string{}, - Log: log.Log, + Log: util.NewLogger(), Arch: "amd64", OLMCatalogPlacement: hyperv1.ManagementOLMCatalogPlacement, NetworkType: string(hyperv1.OVNKubernetes), diff --git a/cmd/cluster/core/destroy_test.go b/cmd/cluster/core/destroy_test.go index 77d8f94c0695..0b9787b4fa5a 100644 --- a/cmd/cluster/core/destroy_test.go +++ b/cmd/cluster/core/destroy_test.go @@ -7,7 +7,7 @@ import ( . "github.com/onsi/gomega" - "github.com/openshift/hypershift/cmd/log" + cmdutil "github.com/openshift/hypershift/cmd/util" ) func TestDestroyCluster(t *testing.T) { @@ -28,7 +28,7 @@ func TestDestroyCluster(t *testing.T) { Name: "test-cluster", Namespace: "clusters", InfraID: "test-infra", - Log: log.Log, + Log: cmdutil.NewLogger(), AzurePlatform: AzurePlatformDestroyOptions{ Cloud: "AzurePublicCloud", Location: "eastus", @@ -59,7 +59,7 @@ func TestDestroyCluster(t *testing.T) { Name: "test-cluster", Namespace: "clusters", InfraID: "test-infra", - Log: log.Log, + Log: cmdutil.NewLogger(), } err := DestroyCluster(context.Background(), nil, opts, mockPlatformSpecifics) diff --git a/cmd/cluster/core/dump.go b/cmd/cluster/core/dump.go index e36e2434da79..e159d836786a 100644 --- a/cmd/cluster/core/dump.go +++ b/cmd/cluster/core/dump.go @@ -15,7 +15,6 @@ import ( hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" hyperkarpenterv1 "github.com/openshift/hypershift/api/karpenter/v1" scheduling "github.com/openshift/hypershift/api/scheduling/v1alpha1" - "github.com/openshift/hypershift/cmd/log" "github.com/openshift/hypershift/cmd/util" "github.com/openshift/hypershift/hypershift-operator/controllers/manifests" "github.com/openshift/hypershift/hypershift-operator/controllers/sharedingress" @@ -150,7 +149,7 @@ func NewDumpCommand() *cobra.Command { ArtifactDir: "", ArchiveDump: true, AgentNamespace: "", - Log: log.Log, + Log: util.NewLogger(), } cmd.Flags().StringVar(&opts.Namespace, "namespace", opts.Namespace, "The namespace of the hostedcluster to dump") diff --git a/cmd/cluster/gcp/destroy.go b/cmd/cluster/gcp/destroy.go index eecb533481e2..211a51fb4a9c 100644 --- a/cmd/cluster/gcp/destroy.go +++ b/cmd/cluster/gcp/destroy.go @@ -4,7 +4,7 @@ import ( "context" "github.com/openshift/hypershift/cmd/cluster/core" - "github.com/openshift/hypershift/cmd/log" + cmdutil "github.com/openshift/hypershift/cmd/util" "github.com/spf13/cobra" ) @@ -17,7 +17,7 @@ func NewDestroyCommand(opts *core.DestroyOptions) *cobra.Command { SilenceUsage: true, } - logger := log.Log + logger := cmdutil.NewLogger() cmd.RunE = func(cmd *cobra.Command, args []string) error { if err := DestroyCluster(cmd.Context(), opts); err != nil { logger.Error(err, "Failed to destroy cluster") diff --git a/cmd/cluster/kubevirt/destroy.go b/cmd/cluster/kubevirt/destroy.go index d277c8abcac4..efd9ffcdc753 100644 --- a/cmd/cluster/kubevirt/destroy.go +++ b/cmd/cluster/kubevirt/destroy.go @@ -3,7 +3,7 @@ package kubevirt import ( "github.com/openshift/hypershift/cmd/cluster/core" "github.com/openshift/hypershift/cmd/cluster/none" - "github.com/openshift/hypershift/cmd/log" + cmdutil "github.com/openshift/hypershift/cmd/util" "github.com/spf13/cobra" ) @@ -15,7 +15,7 @@ func NewDestroyCommand(opts *core.DestroyOptions) *cobra.Command { SilenceUsage: true, } - logger := log.Log + logger := cmdutil.NewLogger() cmd.RunE = func(cmd *cobra.Command, args []string) error { if err := none.DestroyCluster(cmd.Context(), opts); err != nil { logger.Error(err, "Failed to destroy cluster") diff --git a/cmd/cluster/none/destroy.go b/cmd/cluster/none/destroy.go index 889abc84f196..d475d0efdd24 100644 --- a/cmd/cluster/none/destroy.go +++ b/cmd/cluster/none/destroy.go @@ -5,7 +5,7 @@ import ( "fmt" "github.com/openshift/hypershift/cmd/cluster/core" - "github.com/openshift/hypershift/cmd/log" + cmdutil "github.com/openshift/hypershift/cmd/util" "k8s.io/apimachinery/pkg/util/errors" @@ -19,7 +19,7 @@ func NewDestroyCommand(opts *core.DestroyOptions) *cobra.Command { SilenceUsage: true, } - logger := log.Log + logger := cmdutil.NewLogger() cmd.RunE = func(cmd *cobra.Command, args []string) error { if err := DestroyCluster(cmd.Context(), opts); err != nil { logger.Error(err, "Failed to destroy cluster") diff --git a/cmd/cluster/openstack/destroy.go b/cmd/cluster/openstack/destroy.go index 5b73a1d54322..5d1c5e60cf2b 100644 --- a/cmd/cluster/openstack/destroy.go +++ b/cmd/cluster/openstack/destroy.go @@ -8,7 +8,7 @@ import ( "syscall" "github.com/openshift/hypershift/cmd/cluster/core" - "github.com/openshift/hypershift/cmd/log" + cmdutil "github.com/openshift/hypershift/cmd/util" "k8s.io/apimachinery/pkg/util/errors" @@ -22,7 +22,7 @@ func NewDestroyCommand(opts *core.DestroyOptions) *cobra.Command { SilenceUsage: true, } - logger := log.Log + logger := cmdutil.NewLogger() cmd.Run = func(cmd *cobra.Command, args []string) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() diff --git a/cmd/cluster/powervs/destroy.go b/cmd/cluster/powervs/destroy.go index 99bf7a7ccfe1..a9e3a877d90e 100644 --- a/cmd/cluster/powervs/destroy.go +++ b/cmd/cluster/powervs/destroy.go @@ -9,7 +9,7 @@ import ( "github.com/openshift/hypershift/cmd/cluster/core" powervsinfra "github.com/openshift/hypershift/cmd/infra/powervs" - "github.com/openshift/hypershift/cmd/log" + cmdutil "github.com/openshift/hypershift/cmd/util" "k8s.io/apimachinery/pkg/util/errors" @@ -48,7 +48,7 @@ func NewDestroyCommand(opts *core.DestroyOptions) *cobra.Command { _ = cmd.Flags().MarkHidden("vpc") _ = cmd.Flags().MarkHidden("transit-gateway") - logger := log.Log + logger := cmdutil.NewLogger() cmd.Run = func(cmd *cobra.Command, args []string) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() diff --git a/cmd/consolelogs/aws/getlogs.go b/cmd/consolelogs/aws/getlogs.go index 660d6c56df8c..48965fca8413 100644 --- a/cmd/consolelogs/aws/getlogs.go +++ b/cmd/consolelogs/aws/getlogs.go @@ -10,8 +10,7 @@ import ( hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" awsutil "github.com/openshift/hypershift/cmd/infra/aws/util" - "github.com/openshift/hypershift/cmd/log" - "github.com/openshift/hypershift/cmd/util" + cmdutil "github.com/openshift/hypershift/cmd/util" "github.com/openshift/hypershift/support/awsapi" "github.com/aws/aws-sdk-go-v2/aws" @@ -52,7 +51,7 @@ func NewCommand() *cobra.Command { _ = cmd.MarkFlagRequired("name") _ = cmd.MarkFlagRequired("output-dir") - logger := log.Log + logger := cmdutil.NewLogger() cmd.RunE = func(cmd *cobra.Command, args []string) error { err := opts.AWSCredentialsOpts.Validate() if err != nil { @@ -70,7 +69,7 @@ func NewCommand() *cobra.Command { } func (o *ConsoleLogOpts) Run(ctx context.Context) error { - c, err := util.GetClient() + c, err := cmdutil.GetClient() if err != nil { return err } diff --git a/cmd/infra/aws/create.go b/cmd/infra/aws/create.go index 1c2e35eaca73..f5c64a736258 100644 --- a/cmd/infra/aws/create.go +++ b/cmd/infra/aws/create.go @@ -11,8 +11,7 @@ import ( "time" awsutil "github.com/openshift/hypershift/cmd/infra/aws/util" - "github.com/openshift/hypershift/cmd/log" - "github.com/openshift/hypershift/cmd/util" + cmdutil "github.com/openshift/hypershift/cmd/util" "github.com/openshift/hypershift/support/awsapi" "github.com/aws/aws-sdk-go-v2/aws" @@ -47,7 +46,7 @@ type CreateInfraOptions struct { SingleNATGateway bool VPCCIDR string - CredentialsSecretData *util.CredentialsSecretData + CredentialsSecretData *cmdutil.CredentialsSecretData VPCOwnerCredentialOpts awsutil.AWSCredentialsOptions PrivateZonesInClusterAccount bool @@ -129,7 +128,7 @@ func NewCreateCommand() *cobra.Command { opts.AWSCredentialsOpts.BindFlags(cmd.Flags()) opts.VPCOwnerCredentialOpts.BindVPCOwnerFlags(cmd.Flags()) - l := log.Log + l := cmdutil.NewLogger() cmd.RunE = func(cmd *cobra.Command, args []string) error { err := opts.AWSCredentialsOpts.Validate() if err != nil { @@ -528,7 +527,7 @@ func (o *CreateInfraOptions) createProxyHost(ctx context.Context, l logr.Logger, var result proxyInfo - publicSSHKey, privateSSHKey, err := util.GenerateSSHKeys() + publicSSHKey, privateSSHKey, err := cmdutil.GenerateSSHKeys() if err != nil { return nil, fmt.Errorf("failed to generate proxy ssh keys: %w", err) } diff --git a/cmd/infra/aws/create_cli_role.go b/cmd/infra/aws/create_cli_role.go index 22a7d73b0072..fcfa524b54b8 100644 --- a/cmd/infra/aws/create_cli_role.go +++ b/cmd/infra/aws/create_cli_role.go @@ -5,7 +5,7 @@ import ( "fmt" awsutil "github.com/openshift/hypershift/cmd/infra/aws/util" - "github.com/openshift/hypershift/cmd/log" + cmdutil "github.com/openshift/hypershift/cmd/util" "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/service/iam" @@ -175,7 +175,9 @@ func NewCreateCLIRoleCommand() *cobra.Command { cmd.Flags().StringVar(&opts.RoleName, "name", opts.RoleName, "Role name") cmd.Flags().StringToStringVarP(&opts.AdditionalTags, "additional-tags", "t", opts.AdditionalTags, "Additional tags to apply to the role created (e.g. 'key1=value1,key2=value2')") - logger := log.Log + _ = cmd.MarkFlagRequired("aws-creds") + + logger := cmdutil.NewLogger() cmd.RunE = func(cmd *cobra.Command, args []string) error { if err := opts.Run(cmd.Context(), logger); err != nil { logger.Error(err, "failed to create cli role") diff --git a/cmd/infra/aws/create_iam.go b/cmd/infra/aws/create_iam.go index 87f5c88a5d6d..29483dbd72bf 100644 --- a/cmd/infra/aws/create_iam.go +++ b/cmd/infra/aws/create_iam.go @@ -9,8 +9,7 @@ import ( hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" awsutil "github.com/openshift/hypershift/cmd/infra/aws/util" - "github.com/openshift/hypershift/cmd/log" - "github.com/openshift/hypershift/cmd/util" + cmdutil "github.com/openshift/hypershift/cmd/util" "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/service/iam" @@ -43,7 +42,7 @@ type CreateIAMOptions struct { VPCOwnerCredentialsOpts awsutil.AWSCredentialsOptions PrivateZonesInClusterAccount bool - CredentialsSecretData *util.CredentialsSecretData + CredentialsSecretData *cmdutil.CredentialsSecretData additionalIAMTags []iamtypes.Tag CreateKarpenterRoleARN bool @@ -104,13 +103,13 @@ func NewCreateIAMCommand() *cobra.Command { _ = cmd.MarkFlagRequired("oidc-bucket-name") _ = cmd.MarkFlagRequired("oidc-bucket-region") - logger := log.Log + logger := cmdutil.NewLogger() cmd.RunE = func(cmd *cobra.Command, args []string) error { err := opts.AWSCredentialsOpts.Validate() if err != nil { return err } - client, err := util.GetClient() + client, err := cmdutil.GetClient() if err != nil { logger.Error(err, "failed to create client") return err @@ -245,7 +244,7 @@ func (o *CreateIAMOptions) CreateIAM(ctx context.Context, client crclient.Client } func (o *CreateIAMOptions) ParseAdditionalTags() error { - parsed, err := util.ParseAWSTags(o.AdditionalTags) + parsed, err := cmdutil.ParseAWSTags(o.AdditionalTags) if err != nil { return err } diff --git a/cmd/infra/aws/create_operator_roles.go b/cmd/infra/aws/create_operator_roles.go index 99f4c9f2b79a..0b1fee3adb4d 100644 --- a/cmd/infra/aws/create_operator_roles.go +++ b/cmd/infra/aws/create_operator_roles.go @@ -8,7 +8,6 @@ import ( "strings" awsutil "github.com/openshift/hypershift/cmd/infra/aws/util" - "github.com/openshift/hypershift/cmd/log" "github.com/openshift/hypershift/cmd/util" "github.com/openshift/hypershift/support/awsapi" @@ -78,7 +77,7 @@ func NewCreateOperatorRolesCommand() *cobra.Command { _ = cmd.MarkFlagRequired("oidc-storage-provider-s3-bucket-name") - logger := log.Log + logger := util.NewLogger() cmd.RunE = func(cmd *cobra.Command, args []string) error { if err := opts.Validate(cmd.Context()); err != nil { return err diff --git a/cmd/infra/aws/destroy.go b/cmd/infra/aws/destroy.go index 871abcc707bc..d5c2961acdcf 100644 --- a/cmd/infra/aws/destroy.go +++ b/cmd/infra/aws/destroy.go @@ -8,8 +8,7 @@ import ( "time" awsutil "github.com/openshift/hypershift/cmd/infra/aws/util" - "github.com/openshift/hypershift/cmd/log" - "github.com/openshift/hypershift/cmd/util" + cmdutil "github.com/openshift/hypershift/cmd/util" "github.com/openshift/hypershift/support/awsapi" "github.com/aws/aws-sdk-go-v2/aws" @@ -42,7 +41,7 @@ type DestroyInfraOptions struct { AwsInfraGracePeriod time.Duration Log logr.Logger - CredentialsSecretData *util.CredentialsSecretData + CredentialsSecretData *cmdutil.CredentialsSecretData AWSEbsCsiDriverControllerCredentialsFile string CloudControllerCredentialsFile string @@ -124,7 +123,7 @@ func NewDestroyCommand() *cobra.Command { opts := DestroyInfraOptions{ Region: "us-east-1", Name: "example", - Log: log.Log, + Log: cmdutil.NewLogger(), AWSCredentialsOpts: DefaultDelegatedAWSCredentialOptions(), } diff --git a/cmd/infra/aws/destroy_iam.go b/cmd/infra/aws/destroy_iam.go index 1f70facca8af..6b17c029d811 100644 --- a/cmd/infra/aws/destroy_iam.go +++ b/cmd/infra/aws/destroy_iam.go @@ -8,8 +8,7 @@ import ( "time" awsutil "github.com/openshift/hypershift/cmd/infra/aws/util" - "github.com/openshift/hypershift/cmd/log" - "github.com/openshift/hypershift/cmd/util" + cmdutil "github.com/openshift/hypershift/cmd/util" "github.com/openshift/hypershift/support/awsapi" "github.com/aws/aws-sdk-go-v2/aws" @@ -32,7 +31,7 @@ type DestroyIAMOptions struct { VPCOwnerCredentialsOpts awsutil.AWSCredentialsOptions PrivateZonesInClusterAccount bool - CredentialsSecretData *util.CredentialsSecretData + CredentialsSecretData *cmdutil.CredentialsSecretData } func NewDestroyIAMCommand() *cobra.Command { @@ -45,7 +44,7 @@ func NewDestroyIAMCommand() *cobra.Command { opts := DestroyIAMOptions{ Region: "us-east-1", InfraID: "", - Log: log.Log, + Log: cmdutil.NewLogger(), } cmd.Flags().StringVar(&opts.InfraID, "infra-id", opts.InfraID, "Infrastructure ID to use for AWS resources.") diff --git a/cmd/infra/azure/create.go b/cmd/infra/azure/create.go index b41c757e9500..f150507e6c97 100644 --- a/cmd/infra/azure/create.go +++ b/cmd/infra/azure/create.go @@ -8,8 +8,7 @@ import ( "strings" hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" - "github.com/openshift/hypershift/cmd/log" - "github.com/openshift/hypershift/cmd/util" + cmdutil "github.com/openshift/hypershift/cmd/util" "github.com/openshift/hypershift/support/azureutil" "github.com/openshift/hypershift/support/config" @@ -56,16 +55,16 @@ func NewCreateCommand() *cobra.Command { cmd.Flags().StringVar(&opts.WorkloadIdentitiesFile, "workload-identities-file", opts.WorkloadIdentitiesFile, "Path to file containing self-managed Azure workload identities JSON") // RBAC and identity role assignment flags - cmd.Flags().BoolVar(&opts.AssignServicePrincipalRoles, "assign-identity-roles", opts.AssignServicePrincipalRoles, util.AssignIdentityRolesDescription) - cmd.Flags().StringVar(&opts.DNSZoneRG, "dns-zone-rg-name", opts.DNSZoneRG, util.DNSZoneRGNameDescription) - cmd.Flags().BoolVar(&opts.AssignCustomHCPRoles, "assign-custom-hcp-roles", opts.AssignCustomHCPRoles, util.AssignCustomHCPRolesDescription) - cmd.Flags().StringSliceVar(&opts.DisableClusterCapabilities, "disable-cluster-capabilities", opts.DisableClusterCapabilities, util.DisableClusterCapabilitiesDescription) + cmd.Flags().BoolVar(&opts.AssignServicePrincipalRoles, "assign-identity-roles", opts.AssignServicePrincipalRoles, cmdutil.AssignIdentityRolesDescription) + cmd.Flags().StringVar(&opts.DNSZoneRG, "dns-zone-rg-name", opts.DNSZoneRG, cmdutil.DNSZoneRGNameDescription) + cmd.Flags().BoolVar(&opts.AssignCustomHCPRoles, "assign-custom-hcp-roles", opts.AssignCustomHCPRoles, cmdutil.AssignCustomHCPRolesDescription) + cmd.Flags().StringSliceVar(&opts.DisableClusterCapabilities, "disable-cluster-capabilities", opts.DisableClusterCapabilities, cmdutil.DisableClusterCapabilitiesDescription) _ = cmd.MarkFlagRequired("infra-id") _ = cmd.MarkFlagRequired("azure-creds") _ = cmd.MarkFlagRequired("name") - l := log.Log + l := cmdutil.NewLogger() cmd.RunE = func(cmd *cobra.Command, args []string) error { if err := opts.Validate(); err != nil { return err @@ -93,35 +92,35 @@ func DefaultOptions() *CreateInfraOptions { // This exposes only the self-managed Azure flags relevant for the productized CLI. func BindProductFlags(opts *CreateInfraOptions, flags *pflag.FlagSet) { // Required flags - flags.StringVar(&opts.InfraID, "infra-id", opts.InfraID, util.InfraIDDescription) - flags.StringVar(&opts.CredentialsFile, "azure-creds", opts.CredentialsFile, util.AzureCredsDescription) + flags.StringVar(&opts.InfraID, "infra-id", opts.InfraID, cmdutil.InfraIDDescription) + flags.StringVar(&opts.CredentialsFile, "azure-creds", opts.CredentialsFile, cmdutil.AzureCredsDescription) flags.StringVar(&opts.Name, "name", opts.Name, "A name for the HostedCluster") // Location and cloud - flags.StringVar(&opts.Location, "location", opts.Location, util.LocationDescription) + flags.StringVar(&opts.Location, "location", opts.Location, cmdutil.LocationDescription) flags.StringVar(&opts.Cloud, "cloud", opts.Cloud, "Azure cloud environment (AzurePublicCloud, AzureUSGovernmentCloud, AzureChinaCloud)") - flags.StringVar(&opts.BaseDomain, "base-domain", opts.BaseDomain, util.BaseDomainInfraDescription) + flags.StringVar(&opts.BaseDomain, "base-domain", opts.BaseDomain, cmdutil.BaseDomainInfraDescription) // Resource group and tags - flags.StringVar(&opts.ResourceGroupName, "resource-group-name", opts.ResourceGroupName, util.ResourceGroupNameDescription) - flags.StringToStringVarP(&opts.ResourceGroupTags, "resource-group-tags", "t", opts.ResourceGroupTags, util.ResourceGroupTagsDescription) + flags.StringVar(&opts.ResourceGroupName, "resource-group-name", opts.ResourceGroupName, cmdutil.ResourceGroupNameDescription) + flags.StringToStringVarP(&opts.ResourceGroupTags, "resource-group-tags", "t", opts.ResourceGroupTags, cmdutil.ResourceGroupTagsDescription) // Networking - flags.StringVar(&opts.VnetID, "vnet-id", opts.VnetID, util.VnetIDDescription) - flags.StringVar(&opts.SubnetID, "subnet-id", opts.SubnetID, util.SubnetIDDescription) - flags.StringVar(&opts.NetworkSecurityGroupID, "network-security-group-id", opts.NetworkSecurityGroupID, util.NetworkSecurityGroupIDDescription) + flags.StringVar(&opts.VnetID, "vnet-id", opts.VnetID, cmdutil.VnetIDDescription) + flags.StringVar(&opts.SubnetID, "subnet-id", opts.SubnetID, cmdutil.SubnetIDDescription) + flags.StringVar(&opts.NetworkSecurityGroupID, "network-security-group-id", opts.NetworkSecurityGroupID, cmdutil.NetworkSecurityGroupIDDescription) // Self-managed Azure identity flags - flags.StringVar(&opts.WorkloadIdentitiesFile, "workload-identities-file", opts.WorkloadIdentitiesFile, util.WorkloadIdentitiesFileDescription) + flags.StringVar(&opts.WorkloadIdentitiesFile, "workload-identities-file", opts.WorkloadIdentitiesFile, cmdutil.WorkloadIdentitiesFileDescription) // RBAC and role assignment - flags.BoolVar(&opts.AssignServicePrincipalRoles, "assign-identity-roles", opts.AssignServicePrincipalRoles, util.AssignIdentityRolesDescription) - flags.StringVar(&opts.DNSZoneRG, "dns-zone-rg-name", opts.DNSZoneRG, util.DNSZoneRGNameDescription) - flags.BoolVar(&opts.AssignCustomHCPRoles, "assign-custom-hcp-roles", opts.AssignCustomHCPRoles, util.AssignCustomHCPRolesDescription) - flags.StringSliceVar(&opts.DisableClusterCapabilities, "disable-cluster-capabilities", opts.DisableClusterCapabilities, util.DisableClusterCapabilitiesDescription) + flags.BoolVar(&opts.AssignServicePrincipalRoles, "assign-identity-roles", opts.AssignServicePrincipalRoles, cmdutil.AssignIdentityRolesDescription) + flags.StringVar(&opts.DNSZoneRG, "dns-zone-rg-name", opts.DNSZoneRG, cmdutil.DNSZoneRGNameDescription) + flags.BoolVar(&opts.AssignCustomHCPRoles, "assign-custom-hcp-roles", opts.AssignCustomHCPRoles, cmdutil.AssignCustomHCPRolesDescription) + flags.StringSliceVar(&opts.DisableClusterCapabilities, "disable-cluster-capabilities", opts.DisableClusterCapabilities, cmdutil.DisableClusterCapabilitiesDescription) // Output - flags.StringVar(&opts.OutputFile, "output-file", opts.OutputFile, util.InfraOutputFileDescription) + flags.StringVar(&opts.OutputFile, "output-file", opts.OutputFile, cmdutil.InfraOutputFileDescription) } // Run is the main function responsible for creating the Azure infrastructure resources for a HostedCluster. @@ -136,7 +135,7 @@ func (o *CreateInfraOptions) Run(ctx context.Context, l logr.Logger) (*CreateInf BaseDomain: o.BaseDomain, } - subscriptionID, azureCreds, err := util.SetupAzureCredentials(l, o.Credentials, o.CredentialsFile) + subscriptionID, azureCreds, err := cmdutil.SetupAzureCredentials(l, o.Credentials, o.CredentialsFile) if err != nil { return nil, fmt.Errorf("failed to setup Azure credentials: %w", err) } diff --git a/cmd/infra/azure/create_iam.go b/cmd/infra/azure/create_iam.go index 4864236006a1..b1c18c86d263 100644 --- a/cmd/infra/azure/create_iam.go +++ b/cmd/infra/azure/create_iam.go @@ -8,8 +8,7 @@ import ( "net/http" "os" - "github.com/openshift/hypershift/cmd/log" - "github.com/openshift/hypershift/cmd/util" + cmdutil "github.com/openshift/hypershift/cmd/util" "github.com/openshift/hypershift/support/azureutil" "github.com/openshift/hypershift/support/config" @@ -35,15 +34,15 @@ func NewCreateIAMCommand() *cobra.Command { opts := DefaultCreateIAMOptions() - cmd.Flags().StringVar(&opts.Name, "name", opts.Name, util.NameDescription) - cmd.Flags().StringVar(&opts.InfraID, "infra-id", opts.InfraID, util.InfraIDDescription) - cmd.Flags().StringVar(&opts.CredentialsFile, "azure-creds", opts.CredentialsFile, util.AzureCredsDescription) - cmd.Flags().StringVar(&opts.Location, "location", opts.Location, util.LocationDescription) - cmd.Flags().StringVar(&opts.ResourceGroupName, "resource-group-name", opts.ResourceGroupName, util.ResourceGroupNameDescription) - cmd.Flags().StringVar(&opts.OIDCIssuerURL, "oidc-issuer-url", opts.OIDCIssuerURL, util.OIDCIssuerURLDescription) - cmd.Flags().StringVar(&opts.OutputFile, "output-file", opts.OutputFile, util.WorkloadIdentitiesOutputFileDescription) - cmd.Flags().StringVar(&opts.Cloud, "cloud", opts.Cloud, util.CloudDescription) - cmd.Flags().BoolVar(&opts.EnableKMS, "enable-kms", opts.EnableKMS, util.EnableKMSDescription) + cmd.Flags().StringVar(&opts.Name, "name", opts.Name, cmdutil.NameDescription) + cmd.Flags().StringVar(&opts.InfraID, "infra-id", opts.InfraID, cmdutil.InfraIDDescription) + cmd.Flags().StringVar(&opts.CredentialsFile, "azure-creds", opts.CredentialsFile, cmdutil.AzureCredsDescription) + cmd.Flags().StringVar(&opts.Location, "location", opts.Location, cmdutil.LocationDescription) + cmd.Flags().StringVar(&opts.ResourceGroupName, "resource-group-name", opts.ResourceGroupName, cmdutil.ResourceGroupNameDescription) + cmd.Flags().StringVar(&opts.OIDCIssuerURL, "oidc-issuer-url", opts.OIDCIssuerURL, cmdutil.OIDCIssuerURLDescription) + cmd.Flags().StringVar(&opts.OutputFile, "output-file", opts.OutputFile, cmdutil.WorkloadIdentitiesOutputFileDescription) + cmd.Flags().StringVar(&opts.Cloud, "cloud", opts.Cloud, cmdutil.CloudDescription) + cmd.Flags().BoolVar(&opts.EnableKMS, "enable-kms", opts.EnableKMS, cmdutil.EnableKMSDescription) _ = cmd.MarkFlagRequired("name") _ = cmd.MarkFlagRequired("infra-id") @@ -52,7 +51,7 @@ func NewCreateIAMCommand() *cobra.Command { _ = cmd.MarkFlagRequired("oidc-issuer-url") _ = cmd.MarkFlagRequired("output-file") - l := log.Log + l := cmdutil.NewLogger() cmd.RunE = func(cmd *cobra.Command, args []string) error { if err := opts.Validate(); err != nil { return err @@ -78,15 +77,15 @@ func DefaultCreateIAMOptions() *CreateIAMOptions { // BindCreateIAMProductFlags binds flags for the product CLI (hcp) IAM create azure command func BindCreateIAMProductFlags(opts *CreateIAMOptions, flags *pflag.FlagSet) { - flags.StringVar(&opts.Name, "name", opts.Name, util.NameDescription) - flags.StringVar(&opts.InfraID, "infra-id", opts.InfraID, util.InfraIDDescription) - flags.StringVar(&opts.CredentialsFile, "azure-creds", opts.CredentialsFile, util.AzureCredsDescription) - flags.StringVar(&opts.Location, "location", opts.Location, util.LocationDescription) - flags.StringVar(&opts.ResourceGroupName, "resource-group-name", opts.ResourceGroupName, util.ResourceGroupNameDescription) - flags.StringVar(&opts.OIDCIssuerURL, "oidc-issuer-url", opts.OIDCIssuerURL, util.OIDCIssuerURLDescription) - flags.StringVar(&opts.OutputFile, "output-file", opts.OutputFile, util.WorkloadIdentitiesOutputFileDescription) - flags.StringVar(&opts.Cloud, "cloud", opts.Cloud, util.CloudDescription) - flags.BoolVar(&opts.EnableKMS, "enable-kms", opts.EnableKMS, util.EnableKMSDescription) + flags.StringVar(&opts.Name, "name", opts.Name, cmdutil.NameDescription) + flags.StringVar(&opts.InfraID, "infra-id", opts.InfraID, cmdutil.InfraIDDescription) + flags.StringVar(&opts.CredentialsFile, "azure-creds", opts.CredentialsFile, cmdutil.AzureCredsDescription) + flags.StringVar(&opts.Location, "location", opts.Location, cmdutil.LocationDescription) + flags.StringVar(&opts.ResourceGroupName, "resource-group-name", opts.ResourceGroupName, cmdutil.ResourceGroupNameDescription) + flags.StringVar(&opts.OIDCIssuerURL, "oidc-issuer-url", opts.OIDCIssuerURL, cmdutil.OIDCIssuerURLDescription) + flags.StringVar(&opts.OutputFile, "output-file", opts.OutputFile, cmdutil.WorkloadIdentitiesOutputFileDescription) + flags.StringVar(&opts.Cloud, "cloud", opts.Cloud, cmdutil.CloudDescription) + flags.BoolVar(&opts.EnableKMS, "enable-kms", opts.EnableKMS, cmdutil.EnableKMSDescription) } // Validate validates the CreateIAMOptions @@ -115,7 +114,7 @@ func (o *CreateIAMOptions) Validate() error { // Run creates the Azure IAM resources (managed identities and federated credentials) func (o *CreateIAMOptions) Run(ctx context.Context, l logr.Logger) error { // Setup Azure credentials - subscriptionID, azureCreds, err := util.SetupAzureCredentials(l, o.Credentials, o.CredentialsFile) + subscriptionID, azureCreds, err := cmdutil.SetupAzureCredentials(l, o.Credentials, o.CredentialsFile) if err != nil { return fmt.Errorf("failed to setup Azure credentials: %w", err) } diff --git a/cmd/infra/azure/destroy.go b/cmd/infra/azure/destroy.go index ecd2a13b1204..972764b2211d 100644 --- a/cmd/infra/azure/destroy.go +++ b/cmd/infra/azure/destroy.go @@ -5,8 +5,7 @@ import ( "fmt" "strings" - "github.com/openshift/hypershift/cmd/log" - "github.com/openshift/hypershift/cmd/util" + cmdutil "github.com/openshift/hypershift/cmd/util" "github.com/openshift/hypershift/support/azureutil" "github.com/openshift/hypershift/support/config" @@ -25,7 +24,7 @@ type DestroyInfraOptions struct { Location string InfraID string CredentialsFile string - Credentials *util.AzureCreds + Credentials *cmdutil.AzureCreds ResourceGroupName string PreserveResourceGroup bool Cloud string @@ -55,7 +54,7 @@ func NewDestroyCommand() *cobra.Command { _ = cmd.MarkFlagRequired("azure-creds") _ = cmd.MarkFlagRequired("name") - logger := log.Log + logger := cmdutil.NewLogger() cmd.RunE = func(cmd *cobra.Command, args []string) error { if err := opts.Run(cmd.Context(), logger); err != nil { logger.Error(err, "Failed to destroy infrastructure") @@ -81,17 +80,17 @@ func DefaultDestroyOptions() *DestroyInfraOptions { // This exposes only the self-managed Azure flags relevant for the productized CLI. func BindDestroyProductFlags(opts *DestroyInfraOptions, flags *pflag.FlagSet) { // Required flags - flags.StringVar(&opts.InfraID, "infra-id", opts.InfraID, util.InfraIDDescription) - flags.StringVar(&opts.CredentialsFile, "azure-creds", opts.CredentialsFile, util.AzureCredsDescription) + flags.StringVar(&opts.InfraID, "infra-id", opts.InfraID, cmdutil.InfraIDDescription) + flags.StringVar(&opts.CredentialsFile, "azure-creds", opts.CredentialsFile, cmdutil.AzureCredsDescription) flags.StringVar(&opts.Name, "name", opts.Name, "A name for the HostedCluster") // Location and cloud - flags.StringVar(&opts.Location, "location", opts.Location, util.LocationDescription) + flags.StringVar(&opts.Location, "location", opts.Location, cmdutil.LocationDescription) flags.StringVar(&opts.Cloud, "cloud", opts.Cloud, "Azure cloud environment (AzurePublicCloud, AzureUSGovernmentCloud, AzureChinaCloud)") // Resource group - flags.StringVar(&opts.ResourceGroupName, "resource-group-name", opts.ResourceGroupName, util.ResourceGroupNameDescription) - flags.BoolVar(&opts.PreserveResourceGroup, "preserve-resource-group", opts.PreserveResourceGroup, util.PreserveResourceGroupDescription) + flags.StringVar(&opts.ResourceGroupName, "resource-group-name", opts.ResourceGroupName, cmdutil.ResourceGroupNameDescription) + flags.BoolVar(&opts.PreserveResourceGroup, "preserve-resource-group", opts.PreserveResourceGroup, cmdutil.PreserveResourceGroupDescription) } // Validate validates the DestroyInfraOptions before running the destroy operation. @@ -116,7 +115,7 @@ func (o *DestroyInfraOptions) Run(ctx context.Context, logger logr.Logger) error var destroyFuture *runtime.Poller[armresources.ResourceGroupsClientDeleteResponse] // Setup subscription ID and Azure credential information - subscriptionID, azureCreds, err := util.SetupAzureCredentials(logger, o.Credentials, o.CredentialsFile) + subscriptionID, azureCreds, err := cmdutil.SetupAzureCredentials(logger, o.Credentials, o.CredentialsFile) if err != nil { return fmt.Errorf("failed to setup Azure credentials: %w", err) } diff --git a/cmd/infra/azure/destroy_iam.go b/cmd/infra/azure/destroy_iam.go index 259054da9082..5f5ab6063d13 100644 --- a/cmd/infra/azure/destroy_iam.go +++ b/cmd/infra/azure/destroy_iam.go @@ -6,8 +6,7 @@ import ( "fmt" "os" - "github.com/openshift/hypershift/cmd/log" - "github.com/openshift/hypershift/cmd/util" + cmdutil "github.com/openshift/hypershift/cmd/util" "github.com/openshift/hypershift/support/config" "github.com/go-logr/logr" @@ -26,13 +25,13 @@ func NewDestroyIAMCommand() *cobra.Command { opts := DefaultDestroyIAMOptions() - cmd.Flags().StringVar(&opts.WorkloadIdentitiesFile, "workload-identities-file", opts.WorkloadIdentitiesFile, util.WorkloadIdentitiesFileDescription) - cmd.Flags().StringVar(&opts.CredentialsFile, "azure-creds", opts.CredentialsFile, util.AzureCredsDestroyDescription) - cmd.Flags().StringVar(&opts.ResourceGroupName, "resource-group-name", opts.ResourceGroupName, util.ResourceGroupNameDestroyDescription) - cmd.Flags().StringVar(&opts.Cloud, "cloud", opts.Cloud, util.CloudDescription) - cmd.Flags().StringVar(&opts.Name, "name", opts.Name, util.NameDescription) - cmd.Flags().StringVar(&opts.InfraID, "infra-id", opts.InfraID, util.InfraIDDescription) - cmd.Flags().StringVar(&opts.DNSZoneRG, "dns-zone-rg-name", opts.DNSZoneRG, util.DNSZoneRGNameDestroyDescription) + cmd.Flags().StringVar(&opts.WorkloadIdentitiesFile, "workload-identities-file", opts.WorkloadIdentitiesFile, cmdutil.WorkloadIdentitiesFileDescription) + cmd.Flags().StringVar(&opts.CredentialsFile, "azure-creds", opts.CredentialsFile, cmdutil.AzureCredsDestroyDescription) + cmd.Flags().StringVar(&opts.ResourceGroupName, "resource-group-name", opts.ResourceGroupName, cmdutil.ResourceGroupNameDestroyDescription) + cmd.Flags().StringVar(&opts.Cloud, "cloud", opts.Cloud, cmdutil.CloudDescription) + cmd.Flags().StringVar(&opts.Name, "name", opts.Name, cmdutil.NameDescription) + cmd.Flags().StringVar(&opts.InfraID, "infra-id", opts.InfraID, cmdutil.InfraIDDescription) + cmd.Flags().StringVar(&opts.DNSZoneRG, "dns-zone-rg-name", opts.DNSZoneRG, cmdutil.DNSZoneRGNameDestroyDescription) _ = cmd.MarkFlagRequired("workload-identities-file") _ = cmd.MarkFlagRequired("azure-creds") @@ -41,7 +40,7 @@ func NewDestroyIAMCommand() *cobra.Command { _ = cmd.MarkFlagRequired("infra-id") _ = cmd.MarkFlagRequired("dns-zone-rg-name") - l := log.Log + l := cmdutil.NewLogger() cmd.RunE = func(cmd *cobra.Command, args []string) error { if err := opts.Validate(); err != nil { return err @@ -66,12 +65,12 @@ func DefaultDestroyIAMOptions() *DestroyIAMOptions { // BindDestroyIAMProductFlags binds flags for the product CLI (hcp) IAM destroy azure command func BindDestroyIAMProductFlags(opts *DestroyIAMOptions, flags *pflag.FlagSet) { - flags.StringVar(&opts.WorkloadIdentitiesFile, "workload-identities-file", opts.WorkloadIdentitiesFile, util.WorkloadIdentitiesFileDescription) - flags.StringVar(&opts.CredentialsFile, "azure-creds", opts.CredentialsFile, util.AzureCredsDestroyDescription) - flags.StringVar(&opts.ResourceGroupName, "resource-group-name", opts.ResourceGroupName, util.ResourceGroupNameDestroyDescription) - flags.StringVar(&opts.Cloud, "cloud", opts.Cloud, util.CloudDescription) - flags.StringVar(&opts.Name, "name", opts.Name, util.NameDescription) - flags.StringVar(&opts.InfraID, "infra-id", opts.InfraID, util.InfraIDDescription) + flags.StringVar(&opts.WorkloadIdentitiesFile, "workload-identities-file", opts.WorkloadIdentitiesFile, cmdutil.WorkloadIdentitiesFileDescription) + flags.StringVar(&opts.CredentialsFile, "azure-creds", opts.CredentialsFile, cmdutil.AzureCredsDestroyDescription) + flags.StringVar(&opts.ResourceGroupName, "resource-group-name", opts.ResourceGroupName, cmdutil.ResourceGroupNameDestroyDescription) + flags.StringVar(&opts.Cloud, "cloud", opts.Cloud, cmdutil.CloudDescription) + flags.StringVar(&opts.Name, "name", opts.Name, cmdutil.NameDescription) + flags.StringVar(&opts.InfraID, "infra-id", opts.InfraID, cmdutil.InfraIDDescription) flags.StringVar(&opts.DNSZoneRG, "dns-zone-rg-name", opts.DNSZoneRG, "The resource group name where the DNS zone resides (used to clean up role assignments)") } @@ -106,7 +105,7 @@ func (o *DestroyIAMOptions) Run(ctx context.Context, l logr.Logger) error { } // Setup Azure credentials - subscriptionID, azureCreds, err := util.SetupAzureCredentials(l, o.Credentials, o.CredentialsFile) + subscriptionID, azureCreds, err := cmdutil.SetupAzureCredentials(l, o.Credentials, o.CredentialsFile) if err != nil { return fmt.Errorf("failed to setup Azure credentials: %w", err) } diff --git a/cmd/infra/azure/rbac.go b/cmd/infra/azure/rbac.go index f330c4dbdc22..a939a8dbd91b 100644 --- a/cmd/infra/azure/rbac.go +++ b/cmd/infra/azure/rbac.go @@ -12,8 +12,7 @@ import ( "strings" hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" - "github.com/openshift/hypershift/cmd/log" - "github.com/openshift/hypershift/cmd/util" + cmdutil "github.com/openshift/hypershift/cmd/util" "github.com/openshift/hypershift/support/azureutil" "github.com/openshift/hypershift/support/config" @@ -183,7 +182,7 @@ func (r *RBACManager) AssignDataPlaneRoles(ctx context.Context, opts *CreateInfr // assignRole assigns a scoped role to the service principal assignee func (r *RBACManager) assignRole(ctx context.Context, client roleAssignmentClient, infraID, component, assigneeID, role, scope string) error { // Generate the role assignment name - roleAssignmentName := util.GenerateRoleAssignmentName(infraID, component, scope) + roleAssignmentName := cmdutil.GenerateRoleAssignmentName(infraID, component, scope) // Generate the role definition ID roleDefinitionID := fmt.Sprintf("/subscriptions/%s/providers/Microsoft.Authorization/roleDefinitions/%s", r.subscriptionID, role) @@ -217,7 +216,7 @@ func (r *RBACManager) assignRole(ctx context.Context, client roleAssignmentClien continue } if strings.EqualFold(*ra.Properties.Scope, scope) && strings.EqualFold(*ra.Properties.RoleDefinitionID, roleDefinitionID) && strings.EqualFold(*ra.Properties.PrincipalID, assigneeID) { - log.Log.Info("Skipping role assignment creation, matching assignment already exists.", "role", role, "assigneeID", assigneeID, "scope", scope) + cmdutil.NewLogger().Info("Skipping role assignment creation, matching assignment already exists.", "role", role, "assigneeID", assigneeID, "scope", scope) return nil } } @@ -233,7 +232,7 @@ func (r *RBACManager) assignRole(ctx context.Context, client roleAssignmentClien existing.Properties.RoleDefinitionID != nil && strings.EqualFold(*existing.Properties.PrincipalID, assigneeID) && strings.EqualFold(*existing.Properties.RoleDefinitionID, roleDefinitionID) { - log.Log.Info("Skipping role assignment creation, role assignment already exists.", "role", role, "assigneeID", assigneeID, "scope", scope) + cmdutil.NewLogger().Info("Skipping role assignment creation, role assignment already exists.", "role", role, "assigneeID", assigneeID, "scope", scope) return nil } // Stale assignment — different principal owns this deterministic name. @@ -241,7 +240,7 @@ func (r *RBACManager) assignRole(ctx context.Context, client roleAssignmentClien if existing.Properties != nil && existing.Properties.PrincipalID != nil { stalePrincipal = *existing.Properties.PrincipalID } - log.Log.Info("Deleting stale role assignment with mismatched principal", + cmdutil.NewLogger().Info("Deleting stale role assignment with mismatched principal", "role", role, "expectedPrincipal", assigneeID, "stalePrincipal", stalePrincipal, "scope", scope) @@ -255,7 +254,7 @@ func (r *RBACManager) assignRole(ctx context.Context, client roleAssignmentClien if respErr.StatusCode == http.StatusNotFound { // proceed to create } else if respErr.StatusCode == http.StatusForbidden || strings.EqualFold(respErr.ErrorCode, "AuthorizationFailed") { - log.Log.Info("Get not permitted; will attempt create and rely on 409 for idempotency.", "role", role, "assigneeID", assigneeID, "scope", scope) + cmdutil.NewLogger().Info("Get not permitted; will attempt create and rely on 409 for idempotency.", "role", role, "assigneeID", assigneeID, "scope", scope) } else { return fmt.Errorf("failed checking role assignment existence: %w", err) } @@ -268,12 +267,12 @@ func (r *RBACManager) assignRole(ctx context.Context, client roleAssignmentClien if err != nil { var respErr *azcore.ResponseError if errors.As(err, &respErr) && (respErr.StatusCode == http.StatusConflict || strings.EqualFold(respErr.ErrorCode, "RoleAssignmentExists")) { - log.Log.Info("Failed role assignment creation, role assignment already exists.", "role", role, "assigneeID", assigneeID, "scope", scope) + cmdutil.NewLogger().Info("Failed role assignment creation, role assignment already exists.", "role", role, "assigneeID", assigneeID, "scope", scope) return nil } return fmt.Errorf("failed to create role assignment: %w", err) } - log.Log.Info("successfully created role assignment", "role", role, "assigneeID", assigneeID, "scope", scope) + cmdutil.NewLogger().Info("successfully created role assignment", "role", role, "assigneeID", assigneeID, "scope", scope) return nil } @@ -316,7 +315,7 @@ func (r *RBACManager) cleanupRoleAssignments(ctx context.Context, l logr.Logger, for _, component := range components { _, scopes := azureutil.GetServicePrincipalScopes(r.subscriptionID, resourceGroupName, nsgResourceGroupName, vnetResourceGroupName, dnsZoneRG, component, assignCustomHCPRoles) for _, scope := range scopes { - name := util.GenerateRoleAssignmentName(infraID, component, scope) + name := cmdutil.GenerateRoleAssignmentName(infraID, component, scope) if err := r.deleteRoleAssignmentByName(ctx, l, client, scope, name, component); err != nil { deleteErrors = append(deleteErrors, err) } @@ -326,7 +325,7 @@ func (r *RBACManager) cleanupRoleAssignments(ctx context.Context, l logr.Logger, // Cleanup data plane role assignments (only scoped to managed RG) managedRG := fmt.Sprintf("/subscriptions/%s/resourceGroups/%s", r.subscriptionID, resourceGroupName) for _, component := range dataPlaneComponents { - name := util.GenerateRoleAssignmentName(infraID, component, managedRG) + name := cmdutil.GenerateRoleAssignmentName(infraID, component, managedRG) if err := r.deleteRoleAssignmentByName(ctx, l, client, managedRG, name, component); err != nil { deleteErrors = append(deleteErrors, err) } diff --git a/cmd/infra/gcp/create_iam.go b/cmd/infra/gcp/create_iam.go index 403c5c28f9ae..7da1a3c50d20 100644 --- a/cmd/infra/gcp/create_iam.go +++ b/cmd/infra/gcp/create_iam.go @@ -6,7 +6,7 @@ import ( "fmt" "os" - "github.com/openshift/hypershift/cmd/log" + cmdutil "github.com/openshift/hypershift/cmd/util" "github.com/go-logr/logr" "github.com/spf13/cobra" @@ -51,7 +51,7 @@ func NewCreateIAMCommand() *cobra.Command { opts := bindOptions(cmd) - logger := log.Log + logger := cmdutil.NewLogger() cmd.PreRunE = func(cmd *cobra.Command, args []string) error { return opts.ValidateInputs() } diff --git a/cmd/infra/gcp/create_infra.go b/cmd/infra/gcp/create_infra.go index 2d8741fc234f..00605eaaab83 100644 --- a/cmd/infra/gcp/create_infra.go +++ b/cmd/infra/gcp/create_infra.go @@ -6,7 +6,7 @@ import ( "fmt" "os" - "github.com/openshift/hypershift/cmd/log" + cmdutil "github.com/openshift/hypershift/cmd/util" "github.com/go-logr/logr" "github.com/spf13/cobra" @@ -65,7 +65,7 @@ func NewCreateCommand() *cobra.Command { _ = cmd.MarkFlagRequired("region") _ = cmd.MarkFlagRequired("infra-id") - logger := log.Log + logger := cmdutil.NewLogger() cmd.PreRunE = func(cmd *cobra.Command, args []string) error { return opts.Validate() } diff --git a/cmd/infra/gcp/destroy_iam.go b/cmd/infra/gcp/destroy_iam.go index 3c4717f64ea5..a9ae927f11b8 100644 --- a/cmd/infra/gcp/destroy_iam.go +++ b/cmd/infra/gcp/destroy_iam.go @@ -4,7 +4,7 @@ import ( "context" "fmt" - "github.com/openshift/hypershift/cmd/log" + cmdutil "github.com/openshift/hypershift/cmd/util" "github.com/go-logr/logr" "github.com/spf13/cobra" @@ -30,7 +30,7 @@ func NewDestroyIAMCommand() *cobra.Command { _ = cmd.MarkFlagRequired(infraIDFlag) _ = cmd.MarkFlagRequired(projectIDFlag) - logger := log.Log + logger := cmdutil.NewLogger() cmd.PreRunE = func(cmd *cobra.Command, args []string) error { return opts.ValidateInputs() } diff --git a/cmd/infra/gcp/destroy_infra.go b/cmd/infra/gcp/destroy_infra.go index 621a3a622e15..aa875272b298 100644 --- a/cmd/infra/gcp/destroy_infra.go +++ b/cmd/infra/gcp/destroy_infra.go @@ -4,7 +4,7 @@ import ( "context" "fmt" - "github.com/openshift/hypershift/cmd/log" + cmdutil "github.com/openshift/hypershift/cmd/util" "github.com/go-logr/logr" "github.com/spf13/cobra" @@ -36,7 +36,7 @@ func NewDestroyCommand() *cobra.Command { _ = cmd.MarkFlagRequired("region") _ = cmd.MarkFlagRequired("infra-id") - logger := log.Log + logger := cmdutil.NewLogger() cmd.PreRunE = func(cmd *cobra.Command, args []string) error { return opts.Validate() } diff --git a/cmd/infra/powervs/create.go b/cmd/infra/powervs/create.go index b787bdd01efd..80d882e6b28c 100644 --- a/cmd/infra/powervs/create.go +++ b/cmd/infra/powervs/create.go @@ -8,7 +8,7 @@ import ( "strings" "time" - hypershiftLog "github.com/openshift/hypershift/cmd/log" + cmdutil "github.com/openshift/hypershift/cmd/util" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/wait" @@ -227,8 +227,8 @@ func NewCreateCommand() *cobra.Command { _ = cmd.MarkFlagRequired("resource-group") _ = cmd.MarkFlagRequired("infra-id") - logger := hypershiftLog.Log.WithName(opts.InfraID) cmd.RunE = func(cmd *cobra.Command, args []string) error { + logger := cmdutil.NewLogger().WithName(opts.InfraID) if err := opts.Run(cmd.Context(), logger); err != nil { logger.Error(err, "Failed to create infrastructure") return err diff --git a/cmd/infra/powervs/destroy.go b/cmd/infra/powervs/destroy.go index 7ba387596f68..dae4628d31a9 100644 --- a/cmd/infra/powervs/destroy.go +++ b/cmd/infra/powervs/destroy.go @@ -12,7 +12,7 @@ import ( "strings" "time" - "github.com/openshift/hypershift/cmd/log" + cmdutil "github.com/openshift/hypershift/cmd/util" "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/wait" @@ -117,8 +117,8 @@ func NewDestroyCommand() *cobra.Command { _ = cmd.Flags().MarkHidden("cloud-instance-id") _ = cmd.Flags().MarkHidden("transit-gateway") - logger := log.Log.WithName(opts.InfraID) cmd.RunE = func(cmd *cobra.Command, args []string) error { + logger := cmdutil.NewLogger().WithName(opts.InfraID) if err := opts.Run(cmd.Context(), logger); err != nil { logger.Error(err, "Failed to destroy infrastructure") return err diff --git a/cmd/log/log.go b/cmd/log/log.go deleted file mode 100644 index 3a814ab31530..000000000000 --- a/cmd/log/log.go +++ /dev/null @@ -1,11 +0,0 @@ -package log - -import ( - "sigs.k8s.io/controller-runtime/pkg/log/zap" - - "go.uber.org/zap/zapcore" -) - -var Log = zap.New(func(o *zap.Options) { - o.TimeEncoder = zapcore.RFC3339TimeEncoder -}) diff --git a/cmd/nodepool/core/create.go b/cmd/nodepool/core/create.go index a623359bb7f3..3f701ea43585 100644 --- a/cmd/nodepool/core/create.go +++ b/cmd/nodepool/core/create.go @@ -6,8 +6,7 @@ import ( "os" hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" - "github.com/openshift/hypershift/cmd/log" - "github.com/openshift/hypershift/cmd/util" + cmdutil "github.com/openshift/hypershift/cmd/util" hyperapi "github.com/openshift/hypershift/support/api" "github.com/openshift/hypershift/support/releaseinfo" "github.com/openshift/hypershift/support/supportedversion" @@ -66,7 +65,7 @@ type NodePoolPlatformCompleter interface { func (o *CreateNodePoolOptions) CreateRunFunc(platformOpts PlatformOptions) func(cmd *cobra.Command, args []string) error { return func(cmd *cobra.Command, args []string) error { if err := o.CreateNodePool(cmd.Context(), platformOpts); err != nil { - log.Log.Error(err, "Failed to create nodepool") + cmdutil.NewLogger().Error(err, "Failed to create nodepool") return err } return nil @@ -87,7 +86,7 @@ func (o *CreateNodePoolOptions) Validate(ctx context.Context, c crclient.Client) } func (o *CreateNodePoolOptions) CreateNodePool(ctx context.Context, platformOpts PlatformOptions) error { - client, err := util.GetClient() + client, err := cmdutil.GetClient() if err != nil { return err } diff --git a/cmd/nodepool/powervs/create.go b/cmd/nodepool/powervs/create.go index fe5c27f88ae0..429cb5da7549 100644 --- a/cmd/nodepool/powervs/create.go +++ b/cmd/nodepool/powervs/create.go @@ -7,8 +7,8 @@ import ( "syscall" hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" - "github.com/openshift/hypershift/cmd/log" "github.com/openshift/hypershift/cmd/nodepool/core" + cmdutil "github.com/openshift/hypershift/cmd/util" "k8s.io/apimachinery/pkg/util/intstr" @@ -52,7 +52,7 @@ func NewCreateCommand(coreOpts *core.CreateNodePoolOptions) *cobra.Command { }() if err := coreOpts.CreateNodePool(ctx, opts); err != nil { - log.Log.Error(err, "Failed to create nodepool") + cmdutil.NewLogger().Error(err, "Failed to create nodepool") os.Exit(1) } } diff --git a/cmd/oadp/backup.go b/cmd/oadp/backup.go index 927cdd881281..9d2e2f1f7c96 100644 --- a/cmd/oadp/backup.go +++ b/cmd/oadp/backup.go @@ -7,8 +7,7 @@ import ( "strings" "time" - "github.com/openshift/hypershift/cmd/log" - "github.com/openshift/hypershift/cmd/util" + cmdutil "github.com/openshift/hypershift/cmd/util" "github.com/openshift/hypershift/support/oadp" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -24,7 +23,7 @@ import ( func NewCreateBackupCommand() *cobra.Command { opts := &CreateOptions{ - Log: log.Log, + Log: cmdutil.NewLogger(), } cmd := &cobra.Command{ @@ -91,7 +90,7 @@ func (o *CreateOptions) RunBackup(ctx context.Context) error { // Client is needed for validations and actual creation if o.Client == nil { var err error - o.Client, err = util.GetClient() + o.Client, err = cmdutil.GetClient() if err != nil { if o.Render { // In render mode, if we can't connect to cluster, we'll still render but skip validations diff --git a/cmd/oadp/restore.go b/cmd/oadp/restore.go index 5134db9761bc..a0622ae1bdab 100644 --- a/cmd/oadp/restore.go +++ b/cmd/oadp/restore.go @@ -5,8 +5,7 @@ import ( "fmt" "strings" - "github.com/openshift/hypershift/cmd/log" - "github.com/openshift/hypershift/cmd/util" + cmdutil "github.com/openshift/hypershift/cmd/util" "github.com/openshift/hypershift/support/netutil" "github.com/openshift/hypershift/support/oadp" @@ -22,7 +21,7 @@ import ( func NewCreateRestoreCommand() *cobra.Command { opts := &CreateOptions{ - Log: log.Log, + Log: cmdutil.NewLogger(), } // CLI flag variables for boolean fields @@ -123,7 +122,7 @@ func (o *CreateOptions) RunRestore(ctx context.Context) error { // Client is needed for validations and actual creation if o.Client == nil { var err error - o.Client, err = util.GetClient() + o.Client, err = cmdutil.GetClient() if err != nil { if o.Render { // In render mode, if we can't connect to cluster, we'll still render but skip validations diff --git a/cmd/oadp/schedule.go b/cmd/oadp/schedule.go index decc65e5d957..77d5b3308184 100644 --- a/cmd/oadp/schedule.go +++ b/cmd/oadp/schedule.go @@ -7,8 +7,7 @@ import ( "strings" "time" - "github.com/openshift/hypershift/cmd/log" - "github.com/openshift/hypershift/cmd/util" + cmdutil "github.com/openshift/hypershift/cmd/util" "github.com/openshift/hypershift/support/oadp" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -20,7 +19,7 @@ import ( func NewCreateScheduleCommand() *cobra.Command { opts := &CreateOptions{ - Log: log.Log, + Log: cmdutil.NewLogger(), } cmd := &cobra.Command{ @@ -122,7 +121,7 @@ func (o *CreateOptions) RunSchedule(ctx context.Context) error { // Step 3: Create kubernetes client if not already created if o.Client == nil { var err error - o.Client, err = util.GetClient() + o.Client, err = cmdutil.GetClient() if err != nil { if o.Render { // In render mode, if we can't connect to cluster, we'll still render but skip validations diff --git a/cmd/oadp/schedule_test.go b/cmd/oadp/schedule_test.go index b7175dd82607..7ca2fcb1c521 100644 --- a/cmd/oadp/schedule_test.go +++ b/cmd/oadp/schedule_test.go @@ -10,7 +10,7 @@ import ( . "github.com/onsi/gomega" hypershiftv1beta1 "github.com/openshift/hypershift/api/hypershift/v1beta1" - "github.com/openshift/hypershift/cmd/log" + cmdutil "github.com/openshift/hypershift/cmd/util" appsv1 "k8s.io/api/apps/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -1127,7 +1127,7 @@ func TestRunSchedule(t *testing.T) { StorageLocation: "default", TTL: 2 * time.Hour, Schedule: "0 2 * * *", - Log: log.Log, + Log: cmdutil.NewLogger(), } tt.setup(t, opts) diff --git a/cmd/util/azure_test.go b/cmd/util/azure_test.go index aa27cf0c2f51..4684f9228b34 100644 --- a/cmd/util/azure_test.go +++ b/cmd/util/azure_test.go @@ -5,8 +5,6 @@ import ( . "github.com/onsi/gomega" - "github.com/openshift/hypershift/cmd/log" - "github.com/Azure/azure-sdk-for-go/sdk/azidentity" ) @@ -45,7 +43,7 @@ func Test_SetupAzureCredentials(t *testing.T) { for name, test := range tests { t.Run(name, func(t *testing.T) { g := NewGomegaWithT(t) - subscriptionID, _, err := SetupAzureCredentials(log.Log, test.credentials, test.credentialsFile) + subscriptionID, _, err := SetupAzureCredentials(NewLogger(), test.credentials, test.credentialsFile) if test.expectedError { g.Expect(err).To(MatchError(test.expectedError)) } else { diff --git a/cmd/util/log.go b/cmd/util/log.go new file mode 100644 index 000000000000..ad9b0957e4e1 --- /dev/null +++ b/cmd/util/log.go @@ -0,0 +1,16 @@ +package util + +import ( + "sigs.k8s.io/controller-runtime/pkg/log/zap" + + "github.com/go-logr/logr" + "go.uber.org/zap/zapcore" +) + +// NewLogger creates a logr.Logger backed by zap with RFC3339 timestamps. +// This is the standard logger constructor for CLI commands. +func NewLogger() logr.Logger { + return zap.New(func(o *zap.Options) { + o.TimeEncoder = zapcore.RFC3339TimeEncoder + }) +} diff --git a/control-plane-operator/controllers/awsprivatelink/awsprivatelink_controller.go b/control-plane-operator/controllers/awsprivatelink/awsprivatelink_controller.go index 135663feb30c..a8d54394da59 100644 --- a/control-plane-operator/controllers/awsprivatelink/awsprivatelink_controller.go +++ b/control-plane-operator/controllers/awsprivatelink/awsprivatelink_controller.go @@ -67,7 +67,6 @@ type PrivateServiceObserver struct { client.Client clientset *kubeclient.Clientset - log logr.Logger ControllerName string ServiceNamespace string @@ -102,7 +101,6 @@ func ControllerName(name string) string { } func (r *PrivateServiceObserver) SetupWithManager(ctx context.Context, mgr ctrl.Manager) error { - r.log = ctrl.Log.WithName(r.ControllerName).WithValues("name", r.ServiceName, "namespace", r.ServiceNamespace) var err error r.clientset, err = kubeclient.NewForConfig(mgr.GetConfig()) if err != nil { @@ -131,11 +129,13 @@ func (r *PrivateServiceObserver) SetupWithManager(ctx context.Context, mgr ctrl. } func (r *PrivateServiceObserver) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + log := ctrl.LoggerFrom(ctx) + // Fetch the Service svc, err := r.clientset.CoreV1().Services(req.Namespace).Get(ctx, req.Name, metav1.GetOptions{}) if err != nil { if apierrors.IsNotFound(err) { - r.log.Info("service not found") + log.Info("service not found") return ctrl.Result{}, nil } return ctrl.Result{}, err @@ -162,7 +162,7 @@ func (r *PrivateServiceObserver) Reconcile(ctx context.Context, req ctrl.Request } if len(svc.Status.LoadBalancer.Ingress) == 0 { - r.log.Info("load balancer not provisioned yet") + log.Info("load balancer not provisioned yet") return ctrl.Result{}, nil } awsEndpointService := &hyperv1.AWSEndpointService{ @@ -221,6 +221,8 @@ const ( routerDomain = "apps" ) +const ReconcilerControllerName = "awsendpointservice" + // AWSEndpointServiceReconciler watches AWSEndpointService resources and reconciles // the existence of AWS Endpoints for it in the guest cluster infrastructure. type AWSEndpointServiceReconciler struct { @@ -366,6 +368,7 @@ func (r *AWSEndpointServiceReconciler) SetupWithManager(mgr ctrl.Manager) error r.awsClientBuilder = &clientBuilder{} } _, err := ctrl.NewControllerManagedBy(mgr). + Named(ReconcilerControllerName). For(&hyperv1.AWSEndpointService{}). WithOptions(controller.Options{ RateLimiter: workqueue.NewTypedItemExponentialFailureRateLimiter[reconcile.Request](3*time.Second, 30*time.Second), diff --git a/control-plane-operator/controllers/azureprivatelinkservice/controller.go b/control-plane-operator/controllers/azureprivatelinkservice/controller.go index 883a76814d4e..271885d1c4a6 100644 --- a/control-plane-operator/controllers/azureprivatelinkservice/controller.go +++ b/control-plane-operator/controllers/azureprivatelinkservice/controller.go @@ -175,7 +175,8 @@ type RecordSetsAPI interface { // - Microsoft.Network/privateDnsZones/read, write, delete (DNS zone lifecycle) // - Microsoft.Network/privateDnsZones/virtualNetworkLinks/read, write, delete (VNet link) // - Microsoft.Network/privateDnsZones/A/read, write, delete (A record management) -// +const ReconcilerControllerName = "azureprivatelinkservice" + // Azure SDK client interfaces are used instead of concrete types to enable unit testing. type AzurePrivateLinkServiceReconciler struct { client.Client @@ -192,6 +193,7 @@ type AzurePrivateLinkServiceReconciler struct { // HCP deletion until Azure resource cleanup is complete. func (r *AzurePrivateLinkServiceReconciler) SetupWithManager(mgr ctrl.Manager) error { _, err := ctrl.NewControllerManagedBy(mgr). + Named(ReconcilerControllerName). For(&hyperv1.AzurePrivateLinkService{}). Watches(&hyperv1.HostedControlPlane{}, handler.EnqueueRequestsFromMapFunc( r.mapHCPToAzurePLS(), diff --git a/control-plane-operator/controllers/gcpprivateserviceconnect/observer.go b/control-plane-operator/controllers/gcpprivateserviceconnect/observer.go index 6aa35bafcc3f..9d81264d00bb 100644 --- a/control-plane-operator/controllers/gcpprivateserviceconnect/observer.go +++ b/control-plane-operator/controllers/gcpprivateserviceconnect/observer.go @@ -20,8 +20,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/reconcile" - - "github.com/go-logr/logr" ) const ( @@ -34,8 +32,6 @@ const ( type GCPPrivateServiceObserver struct { client.Client - log logr.Logger - ControllerName string ServiceNamespace string ServiceName string @@ -48,8 +44,6 @@ func ControllerName(name string) string { } func (r *GCPPrivateServiceObserver) SetupWithManager(ctx context.Context, mgr ctrl.Manager) error { - r.log = ctrl.Log.WithName(r.ControllerName).WithValues("name", r.ServiceName, "namespace", r.ServiceNamespace) - return ctrl.NewControllerManagedBy(mgr). Named(r.ControllerName). For(&corev1.Service{}). @@ -60,6 +54,8 @@ func (r *GCPPrivateServiceObserver) SetupWithManager(ctx context.Context, mgr ct } func (r *GCPPrivateServiceObserver) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + log := ctrl.LoggerFrom(ctx) + if req.Name != r.ServiceName { return ctrl.Result{}, nil } @@ -68,7 +64,7 @@ func (r *GCPPrivateServiceObserver) Reconcile(ctx context.Context, req ctrl.Requ svc := &corev1.Service{} if err := r.Get(ctx, req.NamespacedName, svc); err != nil { if apierrors.IsNotFound(err) { - r.log.Info("service not found") + log.Info("service not found") return ctrl.Result{}, nil } return ctrl.Result{}, err @@ -76,14 +72,14 @@ func (r *GCPPrivateServiceObserver) Reconcile(ctx context.Context, req ctrl.Requ // Verify this is an Internal Load Balancer if !isInternalLoadBalancer(svc) { - r.log.Info("service is not Internal LoadBalancer type, skipping", "loadBalancerType", svc.Annotations[gcpLoadBalancerTypeAnnotation]) + log.Info("service is not Internal LoadBalancer type, skipping", "loadBalancerType", svc.Annotations[gcpLoadBalancerTypeAnnotation]) return ctrl.Result{}, nil } // Extract LoadBalancer IP and validate it's ready loadBalancerIP, hasValidIP := k8sutil.ExtractLoadBalancerIP(svc) if !hasValidIP { - r.log.Info("LoadBalancer IP not ready yet") + log.Info("LoadBalancer IP not ready yet") return ctrl.Result{}, nil } diff --git a/control-plane-operator/controllers/gcpprivateserviceconnect/psc_endpoint_controller.go b/control-plane-operator/controllers/gcpprivateserviceconnect/psc_endpoint_controller.go index f3168810ad12..5af606ccbbf3 100644 --- a/control-plane-operator/controllers/gcpprivateserviceconnect/psc_endpoint_controller.go +++ b/control-plane-operator/controllers/gcpprivateserviceconnect/psc_endpoint_controller.go @@ -111,6 +111,8 @@ func (b *gcpClientBuilder) getClient(ctx context.Context) (*compute.Service, err return InitCustomerGCPClient(ctx) } +const ReconcilerControllerName = "gcpprivateserviceconnect" + // GCPPrivateServiceConnectReconciler manages PSC endpoints in customer projects type GCPPrivateServiceConnectReconciler struct { client.Client @@ -121,6 +123,7 @@ type GCPPrivateServiceConnectReconciler struct { // SetupWithManager sets up the controller with the Manager. func (r *GCPPrivateServiceConnectReconciler) SetupWithManager(mgr ctrl.Manager) error { _, err := ctrl.NewControllerManagedBy(mgr). + Named(ReconcilerControllerName). For(&hyperv1.GCPPrivateServiceConnect{}). WithOptions(controller.Options{ RateLimiter: workqueue.NewTypedItemExponentialFailureRateLimiter[reconcile.Request](3*time.Second, 30*time.Second), diff --git a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go index 461e7f482bfc..7cdc2031bd4b 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go +++ b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go @@ -155,6 +155,8 @@ const ( kmsAzureCredentials = "KMSAzureCredentials" ) +const ControllerName = "hostedcontrolplane" + type HostedControlPlaneReconciler struct { client.Client @@ -173,7 +175,6 @@ type HostedControlPlaneReconciler struct { // CertRotationScale determines how quickly we rotate certificates - should only be set faster in testing CertRotationScale time.Duration - Log logr.Logger ReleaseProvider releaseinfo.ProviderWithOpenShiftImageRegistryOverrides UserReleaseProvider releaseinfo.Provider createOrUpdate func(hcp *hyperv1.HostedControlPlane) upsert.CreateOrUpdateFN @@ -192,12 +193,13 @@ type HostedControlPlaneReconciler struct { clock clock.Clock } -func (r *HostedControlPlaneReconciler) SetupWithManager(mgr ctrl.Manager, createOrUpdate upsert.CreateOrUpdateFN, hcp *hyperv1.HostedControlPlane) error { +func (r *HostedControlPlaneReconciler) SetupWithManager(mgr ctrl.Manager, createOrUpdate upsert.CreateOrUpdateFN, hcp *hyperv1.HostedControlPlane, logger logr.Logger) error { if r.clock == nil { r.clock = clock.RealClock{} } r.setup(createOrUpdate) b := ctrl.NewControllerManagedBy(mgr). + Named(ControllerName). For(&hyperv1.HostedControlPlane{}). WithOptions(controller.Options{ RateLimiter: workqueue.NewTypedItemExponentialFailureRateLimiter[reconcile.Request](1*time.Second, 10*time.Second), @@ -222,7 +224,7 @@ func (r *HostedControlPlaneReconciler) SetupWithManager(mgr ctrl.Manager, create uidInput := os.Getenv(controlplaneoperator.DefaultSecurityContextUIDEnvVar) if uidInput == "" { - r.Log.Info("DEFAULT_SECURITY_CONTEXT_UID is not set. This should never happen, unless you are running a HO which doesn't support this CPO") + logger.Info("DEFAULT_SECURITY_CONTEXT_UID is not set. This should never happen, unless you are running a HO which doesn't support this CPO") r.DefaultSecurityContextUID = component.DefaultSecurityContextUID return nil } @@ -370,10 +372,11 @@ func (r *HostedControlPlaneReconciler) eventHandlers(scheme *runtime.Scheme, res } func (r *HostedControlPlaneReconciler) reconcileDeletion(ctx context.Context, hostedControlPlane *hyperv1.HostedControlPlane, originalHostedControlPlane *hyperv1.HostedControlPlane) (ctrl.Result, error) { + log := ctrl.LoggerFrom(ctx) condition := &metav1.Condition{ Type: string(hyperv1.AWSDefaultSecurityGroupDeleted), } - if shouldCleanupCloudResources(r.Log, hostedControlPlane) { + if shouldCleanupCloudResources(log, hostedControlPlane) { if code, destroyErr := r.destroyAWSDefaultSecurityGroup(ctx, hostedControlPlane); destroyErr != nil { condition.Message = "failed to delete AWS default security group" if code == "DependencyViolation" { @@ -389,9 +392,9 @@ func (r *HostedControlPlaneReconciler) reconcileDeletion(ctx context.Context, ho switch code { case "UnauthorizedOperation": - r.Log.Error(destroyErr, "Skipping AWS default security group deletion because of unauthorized operation.") + log.Error(destroyErr, "Skipping AWS default security group deletion because of unauthorized operation.") case "DependencyViolation": - r.Log.Error(destroyErr, "Skipping AWS default security group deletion because of dependency violation.") + log.Error(destroyErr, "Skipping AWS default security group deletion because of dependency violation.") default: return ctrl.Result{}, fmt.Errorf("failed to delete AWS default security group: %w", destroyErr) } @@ -426,6 +429,7 @@ func (r *HostedControlPlaneReconciler) reconcileDeletion(ctx context.Context, ho } func (r *HostedControlPlaneReconciler) reconcileEtcdStatus(ctx context.Context, hostedControlPlane *hyperv1.HostedControlPlane) error { + log := ctrl.LoggerFrom(ctx) newCondition := metav1.Condition{ Type: string(hyperv1.EtcdAvailable), Status: metav1.ConditionUnknown, @@ -433,7 +437,7 @@ func (r *HostedControlPlaneReconciler) reconcileEtcdStatus(ctx context.Context, } switch hostedControlPlane.Spec.Etcd.ManagementType { case hyperv1.Managed: - r.Log.Info("Reconciling etcd cluster status for managed strategy") + log.Info("Reconciling etcd cluster status for managed strategy") sts := manifests.EtcdStatefulSet(hostedControlPlane.Namespace) if err := r.Get(ctx, client.ObjectKeyFromObject(sts), sts); err != nil { if apierrors.IsNotFound(err) { @@ -453,7 +457,7 @@ func (r *HostedControlPlaneReconciler) reconcileEtcdStatus(ctx context.Context, newCondition = *conditionPtr } case hyperv1.Unmanaged: - r.Log.Info("Assuming Etcd cluster is running in unmanaged etcd strategy") + log.Info("Assuming Etcd cluster is running in unmanaged etcd strategy") newCondition = metav1.Condition{ Type: string(hyperv1.EtcdAvailable), Status: metav1.ConditionTrue, @@ -468,7 +472,7 @@ func (r *HostedControlPlaneReconciler) reconcileEtcdStatus(ctx context.Context, hostedControlPlane.Spec.Etcd.Managed != nil && len(hostedControlPlane.Spec.Etcd.Managed.Storage.RestoreSnapshotURL) > 0 { restoreCondition := meta.FindStatusCondition(hostedControlPlane.Status.Conditions, string(hyperv1.EtcdSnapshotRestored)) if restoreCondition == nil { - r.Log.Info("Reconciling etcd cluster restore status") + log.Info("Reconciling etcd cluster restore status") sts := manifests.EtcdStatefulSet(hostedControlPlane.Namespace) if err := r.Get(ctx, client.ObjectKeyFromObject(sts), sts); err == nil { rc := metav1.Condition{} @@ -571,8 +575,8 @@ func (r *HostedControlPlaneReconciler) reconcileDegradedStatus(ctx context.Conte } func (r *HostedControlPlaneReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { - r.Log = ctrl.LoggerFrom(ctx) - r.Log.Info("Reconciling") + log := ctrl.LoggerFrom(ctx) + log.Info("Reconciling") hostedControlPlane := &hyperv1.HostedControlPlane{} err := r.Client.Get(ctx, req.NamespacedName, hostedControlPlane) @@ -598,7 +602,7 @@ func (r *HostedControlPlaneReconciler) Reconcile(ctx context.Context, req ctrl.R } if r.OperateOnReleaseImage != "" && r.OperateOnReleaseImage != util.HCPControlPlaneReleaseImage(hostedControlPlane) { - r.Log.Info("releaseImage is " + util.HCPControlPlaneReleaseImage(hostedControlPlane) + ", but this operator is configured for " + r.OperateOnReleaseImage + ", skipping reconciliation") + log.Info("releaseImage is " + util.HCPControlPlaneReleaseImage(hostedControlPlane) + ", but this operator is configured for " + r.OperateOnReleaseImage + ", skipping reconciliation") return ctrl.Result{}, nil } @@ -686,8 +690,8 @@ func (r *HostedControlPlaneReconciler) Reconcile(ctx context.Context, req ctrl.R if err := r.Client.Status().Patch(ctx, hostedControlPlane, client.MergeFromWithOptions(originalHostedControlPlane, client.MergeFromWithOptimisticLock{})); err != nil { return ctrl.Result{}, fmt.Errorf("failed to update status: %w", err) } - if isPaused, duration := util.IsReconciliationPaused(r.Log, hostedControlPlane.Spec.PausedUntil); isPaused { - r.Log.Info("Reconciliation paused", "pausedUntil", *hostedControlPlane.Spec.PausedUntil) + if isPaused, duration := util.IsReconciliationPaused(log, hostedControlPlane.Spec.PausedUntil); isPaused { + log.Info("Reconciliation paused", "pausedUntil", *hostedControlPlane.Spec.PausedUntil) return ctrl.Result{ RequeueAfter: duration, }, nil @@ -697,7 +701,7 @@ func (r *HostedControlPlaneReconciler) Reconcile(ctx context.Context, req ctrl.R { validConfig := meta.FindStatusCondition(hostedControlPlane.Status.Conditions, string(hyperv1.ValidHostedControlPlaneConfiguration)) if validConfig != nil && validConfig.Status == metav1.ConditionFalse { - r.Log.Info("Configuration is invalid, reconciliation is blocked") + log.Info("Configuration is invalid, reconciliation is blocked") return reconcile.Result{}, nil } } @@ -708,7 +712,7 @@ func (r *HostedControlPlaneReconciler) Reconcile(ctx context.Context, req ctrl.R return ctrl.Result{}, fmt.Errorf("failed to update control plane: %w", err) } - r.Log.Info("Successfully reconciled") + log.Info("Successfully reconciled") if !result.IsZero() { return result, nil @@ -722,7 +726,8 @@ func (r *HostedControlPlaneReconciler) Reconcile(ctx context.Context, req ctrl.R } func (r *HostedControlPlaneReconciler) reconcileInfrastructureStatusCondition(ctx context.Context, hostedControlPlane *hyperv1.HostedControlPlane) { - r.Log.Info("Reconciling infrastructure status") + log := ctrl.LoggerFrom(ctx) + log.Info("Reconciling infrastructure status") newCondition := metav1.Condition{ Type: string(hyperv1.InfrastructureReady), Status: metav1.ConditionUnknown, @@ -736,7 +741,7 @@ func (r *HostedControlPlaneReconciler) reconcileInfrastructureStatusCondition(ct Reason: hyperv1.InfraStatusFailureReason, Message: err.Error(), } - r.Log.Error(err, "failed to determine infrastructure status") + log.Error(err, "failed to determine infrastructure status") } else if infraStatus.IsReady() { hostedControlPlane.Status.ControlPlaneEndpoint = hyperv1.APIEndpoint{ Host: infraStatus.APIHost, @@ -762,7 +767,7 @@ func (r *HostedControlPlaneReconciler) reconcileInfrastructureStatusCondition(ct Reason: hyperv1.WaitingOnInfrastructureReadyReason, Message: message, } - r.Log.Info("Infrastructure is not yet ready") + log.Info("Infrastructure is not yet ready") } newCondition.ObservedGeneration = hostedControlPlane.Generation meta.SetStatusCondition(&hostedControlPlane.Status.Conditions, newCondition) @@ -1078,9 +1083,10 @@ func (r *HostedControlPlaneReconciler) LookupReleaseImage(ctx context.Context, h } func (r *HostedControlPlaneReconciler) update(ctx context.Context, hostedControlPlane *hyperv1.HostedControlPlane, releaseImage *releaseinfo.ReleaseImage) (reconcile.Result, error) { + log := ctrl.LoggerFrom(ctx) createOrUpdate := r.createOrUpdate(hostedControlPlane) - r.Log.Info("Reconciling infrastructure services") + log.Info("Reconciling infrastructure services") if err := r.reconcileInfrastructure(ctx, hostedControlPlane, createOrUpdate); err != nil { return reconcile.Result{}, fmt.Errorf("failed to ensure infrastructure: %w", err) } @@ -1093,7 +1099,7 @@ func (r *HostedControlPlaneReconciler) update(ctx context.Context, hostedControl return reconcile.Result{}, fmt.Errorf("failed to look up infra status: %w", err) } if !infraStatus.IsReady() { - r.Log.Info("Waiting for infrastructure to be ready before proceeding") + log.Info("Waiting for infrastructure to be ready before proceeding") return reconcile.Result{RequeueAfter: time.Minute}, nil } @@ -1150,6 +1156,7 @@ func (r *HostedControlPlaneReconciler) update(ctx context.Context, hostedControl } func (r *HostedControlPlaneReconciler) reconcileCPOV2(ctx context.Context, hcp *hyperv1.HostedControlPlane, infraStatus infra.InfrastructureStatus, releaseImageProvider, userReleaseImageProvider imageprovider.ReleaseImageProvider) error { + log := ctrl.LoggerFrom(ctx) if err := r.cleanupOldKonnectivityServerDeployment(ctx, hcp); err != nil { return err } @@ -1165,21 +1172,21 @@ func (r *HostedControlPlaneReconciler) reconcileCPOV2(ctx context.Context, hcp * for _, resource := range []client.Object{role, sa, roleBinding} { if _, err := k8sutil.DeleteIfNeeded(ctx, r.Client, resource); err != nil { - r.Log.Error(err, "Failed to delete deprecated resource", "resource", client.ObjectKeyFromObject(resource).String()) + log.Error(err, "Failed to delete deprecated resource", "resource", client.ObjectKeyFromObject(resource).String()) } } } createOrUpdate := r.createOrUpdate(hcp) // Reconcile default service account - r.Log.Info("Reconciling default service account") + log.Info("Reconciling default service account") if err := r.reconcileDefaultServiceAccount(ctx, hcp, createOrUpdate); err != nil { return fmt.Errorf("failed to reconcile default service account: %w", err) } // Reconcile PKI if _, exists := hcp.Annotations[hyperv1.DisablePKIReconciliationAnnotation]; !exists { - r.Log.Info("Reconciling PKI") + log.Info("Reconciling PKI") if err := r.reconcilePKI(ctx, hcp, infraStatus, createOrUpdate); err != nil { return fmt.Errorf("failed to reconcile PKI: %w", err) } @@ -1187,7 +1194,7 @@ func (r *HostedControlPlaneReconciler) reconcileCPOV2(ctx context.Context, hcp * // Reconcile unmanaged etcd if hcp.Spec.Etcd.ManagementType == hyperv1.Unmanaged { - r.Log.Info("Reconciling unmanaged Etcd") + log.Info("Reconciling unmanaged Etcd") if err := r.reconcileUnmanagedEtcd(ctx, hcp, createOrUpdate); err != nil { return fmt.Errorf("failed to reconcile etcd: %w", err) } @@ -1213,7 +1220,7 @@ func (r *HostedControlPlaneReconciler) reconcileCPOV2(ctx context.Context, hcp * if _, exists := hcp.Annotations[hyperv1.DisableIgnitionServerAnnotation]; !exists { // Reconcile Ignition-server configs - r.Log.Info("Reconciling ignition-server configs") + log.Info("Reconciling ignition-server configs") if err := r.reconcileIgnitionServerConfigs(ctx, hcp, createOrUpdate); err != nil { return fmt.Errorf("failed to reconcile ignition-server configs: %w", err) } @@ -1221,7 +1228,7 @@ func (r *HostedControlPlaneReconciler) reconcileCPOV2(ctx context.Context, hcp * if util.HCPOAuthEnabled(hcp) { // Reconcile kubeadmin password - r.Log.Info("Reconciling kubeadmin password secret") + log.Info("Reconciling kubeadmin password secret") explicitOauthConfig := hcp.Spec.Configuration != nil && hcp.Spec.Configuration.OAuth != nil if err := r.reconcileKubeadminPassword(ctx, hcp, explicitOauthConfig, createOrUpdate); err != nil { return fmt.Errorf("failed to ensure control plane: %w", err) @@ -1237,7 +1244,7 @@ func (r *HostedControlPlaneReconciler) reconcileCPOV2(ctx context.Context, hcp * return fmt.Errorf("failed to reconcile cluster network operator operands: %w", err) } - r.Log.Info("Reconciling default security group") + log.Info("Reconciling default security group") if err := r.reconcileDefaultSecurityGroup(ctx, hcp); err != nil { return fmt.Errorf("failed to reconcile default security group: %w", err) } @@ -1261,7 +1268,7 @@ func (r *HostedControlPlaneReconciler) reconcileCPOV2(ctx context.Context, hcp * var errs []error for _, c := range r.components { - r.Log.Info("Reconciling component", "component_name", c.Name()) + log.Info("Reconciling component", "component_name", c.Name()) if err := c.Reconcile(cpContext); err != nil { errs = append(errs, err) } @@ -1543,12 +1550,13 @@ func (r *HostedControlPlaneReconciler) reconcileOAuthCerts(ctx context.Context, } func (r *HostedControlPlaneReconciler) reconcileOLMAndMiscCerts(ctx context.Context, hcp *hyperv1.HostedControlPlane, p *pki.PKIParams, createOrUpdate upsert.CreateOrUpdateFN, rootCASecret *corev1.Secret) error { + log := ctrl.LoggerFrom(ctx) if capabilities.IsNodeTuningCapabilityEnabled(hcp.Spec.Capabilities) { NodeTuningOperatorServingCert := manifests.ClusterNodeTuningOperatorServingCertSecret(hcp.Namespace) NodeTuningOperatorService := manifests.ClusterNodeTuningOperatorMetricsService(hcp.Namespace) err := removeServiceCAAnnotationAndSecret(ctx, r.Client, NodeTuningOperatorService, NodeTuningOperatorServingCert) if err != nil { - r.Log.Error(err, "failed to remove service ca annotation and secret: %w") + log.Error(err, "failed to remove service ca annotation and secret") } if _, err = createOrUpdate(ctx, r, NodeTuningOperatorServingCert, func() error { return pki.ReconcileNodeTuningOperatorServingCertSecret(NodeTuningOperatorServingCert, rootCASecret, p.OwnerRef) @@ -1744,6 +1752,7 @@ func (r *HostedControlPlaneReconciler) reconcileAWSPlatformCerts(ctx context.Con } func (r *HostedControlPlaneReconciler) reconcileAzurePlatformCerts(ctx context.Context, hcp *hyperv1.HostedControlPlane, p *pki.PKIParams, createOrUpdate upsert.CreateOrUpdateFN, rootCASecret *corev1.Secret) error { + log := ctrl.LoggerFrom(ctx) azureWorkloadIdentityWebhookServingCert := manifests.AzureWorkloadIdentityWebhookServingCert(hcp.Namespace) if _, err := createOrUpdate(ctx, r, azureWorkloadIdentityWebhookServingCert, func() error { return pki.ReconcileAzureWorkloadIdentityWebhookServingCert(azureWorkloadIdentityWebhookServingCert, rootCASecret, p.OwnerRef) @@ -1754,7 +1763,7 @@ func (r *HostedControlPlaneReconciler) reconcileAzurePlatformCerts(ctx context.C AzureDiskCsiDriverOperatorServingCert := manifests.AzureDiskCSIDriverOperatorServingCertSecret(hcp.Namespace) AzureDiskCsiDriverOperatorService := manifests.AzureDiskCSIDriverOperatorMetricsService(hcp.Namespace) if err := removeServiceCAAnnotationAndSecret(ctx, r.Client, AzureDiskCsiDriverOperatorService, AzureDiskCsiDriverOperatorServingCert); err != nil { - r.Log.Error(err, "failed to remove service ca annotation and secret: %w") + log.Error(err, "failed to remove service ca annotation and secret") } if _, err := createOrUpdate(ctx, r, AzureDiskCsiDriverOperatorServingCert, func() error { z := pki.ReconcileAzureDiskCsiDriverOperatorMetricsServingCertSecret(AzureDiskCsiDriverOperatorServingCert, rootCASecret, p.OwnerRef) @@ -1787,7 +1796,7 @@ func (r *HostedControlPlaneReconciler) reconcileAzurePlatformCerts(ctx context.C AzureFileCsiDriverOperatorServingCert := manifests.AzureFileCSIDriverOperatorServingCertSecret(hcp.Namespace) AzureFileCsiDriverOperatorService := manifests.AzureFileCSIDriverOperatorMetricsService(hcp.Namespace) if err := removeServiceCAAnnotationAndSecret(ctx, r.Client, AzureFileCsiDriverOperatorService, AzureFileCsiDriverOperatorServingCert); err != nil { - r.Log.Error(err, "failed to remove service ca annotation and secret: %w") + log.Error(err, "failed to remove service ca annotation and secret") } if _, err := createOrUpdate(ctx, r, AzureFileCsiDriverOperatorServingCert, func() error { z := pki.ReconcileAzureFileCsiDriverOperatorMetricsServingCertSecret(AzureFileCsiDriverOperatorServingCert, rootCASecret, p.OwnerRef) @@ -1936,11 +1945,12 @@ func (r *HostedControlPlaneReconciler) reconcilePKI(ctx context.Context, hcp *hy } func (r *HostedControlPlaneReconciler) reconcileUnmanagedEtcd(ctx context.Context, hcp *hyperv1.HostedControlPlane, createOrUpdate upsert.CreateOrUpdateFN) error { + log := ctrl.LoggerFrom(ctx) // reconcile client secret over if hcp.Spec.Etcd.Unmanaged == nil || len(hcp.Spec.Etcd.Unmanaged.TLS.ClientSecret.Name) == 0 || len(hcp.Spec.Etcd.Unmanaged.Endpoint) == 0 { return fmt.Errorf("etcd metadata not specified for unmanaged deployment") } - r.Log.Info("Retrieving tls secret", "name", hcp.Spec.Etcd.Unmanaged.TLS.ClientSecret.Name) + log.Info("Retrieving tls secret", "name", hcp.Spec.Etcd.Unmanaged.TLS.ClientSecret.Name) var src corev1.Secret if err := r.Client.Get(ctx, client.ObjectKey{Namespace: hcp.GetNamespace(), Name: hcp.Spec.Etcd.Unmanaged.TLS.ClientSecret.Name}, &src); err != nil { return fmt.Errorf("failed to get etcd client cert %s: %w", hcp.Spec.Etcd.Unmanaged.TLS.ClientSecret.Name, err) @@ -1955,7 +1965,7 @@ func (r *HostedControlPlaneReconciler) reconcileUnmanagedEtcd(ctx context.Contex return fmt.Errorf("etcd secret %s does not have client ca", hcp.Spec.Etcd.Unmanaged.TLS.ClientSecret.Name) } kubeComponentEtcdClientSecret := manifests.EtcdClientSecret(hcp.GetNamespace()) - r.Log.Info("Reconciling openshift control plane etcd client tls secret", "name", kubeComponentEtcdClientSecret.Name) + log.Info("Reconciling openshift control plane etcd client tls secret", "name", kubeComponentEtcdClientSecret.Name) _, err := createOrUpdate(ctx, r.Client, kubeComponentEtcdClientSecret, func() error { if kubeComponentEtcdClientSecret.Data == nil { kubeComponentEtcdClientSecret.Data = map[string][]byte{} @@ -1993,6 +2003,7 @@ func (r *HostedControlPlaneReconciler) cleanupOldPKIOperatorDeployment(ctx conte } func (r *HostedControlPlaneReconciler) reconcileValidIDPConfigurationCondition(ctx context.Context, hcp *hyperv1.HostedControlPlane, releaseImageProvider imageprovider.ReleaseImageProvider, oauthHost string, oauthPort int32) error { + log := ctrl.LoggerFrom(ctx) p := oauth.NewOAuthServerParams(hcp, releaseImageProvider, oauthHost, oauthPort, r.SetDefaultSecurityContext) // Report any IDP configuration errors as a condition on the HCP @@ -2004,7 +2015,7 @@ func (r *HostedControlPlaneReconciler) reconcileValidIDPConfigurationCondition(c } if _, _, err := oauth.ConvertIdentityProviders(ctx, p.IdentityProviders(), p.OauthConfigOverrides, r, hcp.Namespace); err != nil { // Report the error in a condition on the HCP - r.Log.Error(err, "failed to initialize identity providers") + log.Error(err, "failed to initialize identity providers") new = metav1.Condition{ Type: string(hyperv1.ValidIDPConfiguration), Status: metav1.ConditionFalse, @@ -2068,14 +2079,15 @@ func (r *HostedControlPlaneReconciler) cleanupClusterNetworkOperatorResources(ct } func (r *HostedControlPlaneReconciler) reconcileIgnitionServerConfigs(ctx context.Context, hcp *hyperv1.HostedControlPlane, createOrUpdate upsert.CreateOrUpdateFN) error { + log := ctrl.LoggerFrom(ctx) // Reconcile core ignition config - r.Log.Info("Reconciling core ignition config") + log.Info("Reconciling core ignition config") if err := r.reconcileCoreIgnitionConfig(ctx, hcp, createOrUpdate); err != nil { return fmt.Errorf("failed to reconcile core ignition config: %w", err) } // Reconcile machine config server config - r.Log.Info("Reconciling machine config server config") + log.Info("Reconciling machine config server config") if err := r.reconcileMachineConfigServerConfig(ctx, hcp, createOrUpdate); err != nil { return fmt.Errorf("failed to reconcile mcs config: %w", err) } @@ -2139,6 +2151,7 @@ func (r *HostedControlPlaneReconciler) reconcileManagedTrustedCABundle(ctx conte } func (r *HostedControlPlaneReconciler) reconcileCoreIgnitionConfig(ctx context.Context, hcp *hyperv1.HostedControlPlane, createOrUpdate upsert.CreateOrUpdateFN) error { + log := ctrl.LoggerFrom(ctx) sshKey := "" if len(hcp.Spec.SSHKey.Name) > 0 { var sshKeySecret corev1.Secret @@ -2181,7 +2194,7 @@ func (r *HostedControlPlaneReconciler) reconcileCoreIgnitionConfig(ctx context.C } // ImageDigestMirrorSet is only applicable for release image versions >= 4.13 - r.Log.Info("Reconciling ImageDigestMirrorSet") + log.Info("Reconciling ImageDigestMirrorSet") imageDigestMirrorSet := globalconfig.ImageDigestMirrorSet() if err := globalconfig.ReconcileImageDigestMirrors(imageDigestMirrorSet, hcp); err != nil { return fmt.Errorf("failed to reconcile image content policy: %w", err) @@ -2383,15 +2396,16 @@ func reconcileKubeadminPasswordSecret(secret *corev1.Secret, hcp *hyperv1.Hosted } func (r *HostedControlPlaneReconciler) hostedControlPlaneInNamespace(ctx context.Context, resource client.Object) []reconcile.Request { + log := ctrl.LoggerFrom(ctx) hcpList := &hyperv1.HostedControlPlaneList{} if err := r.List(ctx, hcpList, &client.ListOptions{ Namespace: resource.GetNamespace(), }); err != nil { - r.Log.Error(err, "failed to list hosted control planes in namespace", "namespace", resource.GetNamespace()) + log.Error(err, "failed to list hosted control planes in namespace", "namespace", resource.GetNamespace()) return nil } if len(hcpList.Items) > 1 { - r.Log.Error(fmt.Errorf("more than one HostedControlPlane resource found in namespace %s", resource.GetNamespace()), "unexpected number of HostedControlPlane resources") + log.Error(fmt.Errorf("more than one HostedControlPlane resource found in namespace %s", resource.GetNamespace()), "unexpected number of HostedControlPlane resources") return nil } var result []reconcile.Request diff --git a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go index 5bf366b05b19..736401d6deb1 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go +++ b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go @@ -151,7 +151,6 @@ func TestReconcileKubeadminPassword(t *testing.T) { fakeClient := fake.NewClientBuilder().Build() r := &HostedControlPlaneReconciler{ Client: fakeClient, - Log: ctrl.LoggerFrom(t.Context()), } err := r.reconcileKubeadminPassword(t.Context(), tc.hcp, tc.hcp.Spec.Configuration != nil && tc.hcp.Spec.Configuration.OAuth != nil, controllerutil.CreateOrUpdate) g.Expect(err).NotTo(HaveOccurred()) @@ -546,7 +545,6 @@ func TestEtcdRestoredCondition(t *testing.T) { fakeClient := fake.NewClientBuilder().WithLists(podList).Build() r := &HostedControlPlaneReconciler{ Client: fakeClient, - Log: ctrl.LoggerFrom(t.Context()), } conditionPtr := r.etcdRestoredCondition(t.Context(), tc.sts) @@ -1789,7 +1787,6 @@ func TestControlPlaneComponentsAvailable(t *testing.T) { // Create reconciler r := &HostedControlPlaneReconciler{ Client: c, - Log: zapr.NewLogger(zaptest.NewLogger(t)), } // Execute the function under test @@ -2188,7 +2185,6 @@ func TestRemoveHCPIngressFromRoutes(t *testing.T) { r := &HostedControlPlaneReconciler{ Client: fakeClient, - Log: ctrl.LoggerFrom(ctx), ManagementClusterCapabilities: caps, } @@ -2647,7 +2643,6 @@ func TestEtcdStatefulSetCondition(t *testing.T) { r := &HostedControlPlaneReconciler{ Client: fakeClient, - Log: ctrl.LoggerFrom(t.Context()), } condition, err := r.etcdStatefulSetCondition(t.Context(), tc.sts) @@ -3145,7 +3140,6 @@ func TestReconcileEtcdStatus(t *testing.T) { r := &HostedControlPlaneReconciler{ Client: c, - Log: zapr.NewLogger(zaptest.NewLogger(t)), } err := r.reconcileEtcdStatus(t.Context(), tc.hcp) @@ -3363,7 +3357,6 @@ func TestReconcileKASStatus(t *testing.T) { r := &HostedControlPlaneReconciler{ Client: c, - Log: zapr.NewLogger(zaptest.NewLogger(t)), } err := r.reconcileKASStatus(t.Context(), tc.hcp) @@ -3594,7 +3587,6 @@ func TestReconcileDegradedStatus(t *testing.T) { r := &HostedControlPlaneReconciler{ Client: c, - Log: zapr.NewLogger(zaptest.NewLogger(t)), } err := r.reconcileDegradedStatus(t.Context(), tc.hcp) @@ -3712,7 +3704,6 @@ func TestReconcileInfrastructureStatusCondition(t *testing.T) { r := &HostedControlPlaneReconciler{ Client: fake.NewClientBuilder().WithScheme(api.Scheme).Build(), - Log: zapr.NewLogger(zaptest.NewLogger(t)), reconcileInfrastructureStatus: func(ctx context.Context, hcp *hyperv1.HostedControlPlane) (infra.InfrastructureStatus, error) { return tc.infraStatus, tc.infraErr }, @@ -3810,7 +3801,6 @@ func TestReconcileExternalDNSStatusCondition(t *testing.T) { r := &HostedControlPlaneReconciler{ Client: fake.NewClientBuilder().WithScheme(api.Scheme).Build(), - Log: zapr.NewLogger(zaptest.NewLogger(t)), } r.reconcileExternalDNSStatusCondition(t.Context(), tc.hcp) @@ -3943,7 +3933,6 @@ func TestReconcileAvailabilityAndReadyStatus(t *testing.T) { r := &HostedControlPlaneReconciler{ Client: c, - Log: zapr.NewLogger(zaptest.NewLogger(t)), } r.reconcileAvailabilityAndReadyStatus(t.Context(), tc.hcp) @@ -4067,7 +4056,6 @@ func TestReconcileKubeadminPasswordStatus(t *testing.T) { r := &HostedControlPlaneReconciler{ Client: c, - Log: zapr.NewLogger(zaptest.NewLogger(t)), } err := r.reconcileKubeadminPasswordStatus(t.Context(), tc.hcp) @@ -4274,7 +4262,6 @@ func TestReconcileControlPlaneVersionStatus(t *testing.T) { r := &HostedControlPlaneReconciler{ Client: c, - Log: zapr.NewLogger(zaptest.NewLogger(t)), ImageMetadataProvider: imgProvider, clock: fakeClock, } @@ -4424,7 +4411,6 @@ func TestReconcileDeletion(t *testing.T) { r := &HostedControlPlaneReconciler{ Client: fakeClient, - Log: ctrl.Log.WithName("test"), ec2Client: mockEC2, } diff --git a/control-plane-operator/hostedclusterconfigoperator/controllers/cmca/configmap_observer.go b/control-plane-operator/hostedclusterconfigoperator/controllers/cmca/configmap_observer.go index 1bdab4200086..e4c66da245f7 100644 --- a/control-plane-operator/hostedclusterconfigoperator/controllers/cmca/configmap_observer.go +++ b/control-plane-operator/hostedclusterconfigoperator/controllers/cmca/configmap_observer.go @@ -17,8 +17,6 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" - - "github.com/go-logr/logr" ) const ( @@ -72,9 +70,6 @@ type ManagedCAObserver struct { // hcpName is the name of the hostedcontrolplane resource in the // control plane namespace hcpName string - - // log is the logger for this controller - log logr.Logger } // Reconcile periodically watches configmaps in the guest cluster and syncs them to the control plane side @@ -94,7 +89,7 @@ func (r *ManagedCAObserver) Reconcile(ctx context.Context, req ctrl.Request) (ct return ctrl.Result{}, nil } - log := r.log.WithValues("configmap", req.NamespacedName) + log := ctrl.LoggerFrom(ctx) log.Info("syncing configmap") ownerRef := config.OwnerRefFrom(hcp) diff --git a/control-plane-operator/hostedclusterconfigoperator/controllers/cmca/setup.go b/control-plane-operator/hostedclusterconfigoperator/controllers/cmca/setup.go index 3c9d1272a2ef..c90fce163ea6 100644 --- a/control-plane-operator/hostedclusterconfigoperator/controllers/cmca/setup.go +++ b/control-plane-operator/hostedclusterconfigoperator/controllers/cmca/setup.go @@ -19,6 +19,7 @@ import ( const ( ManagedConfigNamespace = "openshift-config-managed" ControllerManagerAdditionalCAConfigMap = "controller-manager-additional-ca" + ControllerName = "ca-configmap-observer" ) func Setup(ctx context.Context, cfg *operator.HostedClusterConfigOperatorConfig) error { @@ -48,10 +49,9 @@ func setupConfigMapObserver(cfg *operator.HostedClusterConfigOperatorConfig) err cmLister: configMaps.Lister(), namespace: cfg.Namespace, hcpName: cfg.HCPName, - log: cfg.Logger.WithName("ManagedCAObserver"), createOrUpdate: upsert.New(cfg.EnableCIDebugOutput).CreateOrUpdate, } - c, err := controller.New("ca-configmap-observer", cfg.Manager, controller.Options{Reconciler: reconciler}) + c, err := controller.New(ControllerName, cfg.Manager, controller.Options{Reconciler: reconciler}) if err != nil { return err } diff --git a/control-plane-operator/main.go b/control-plane-operator/main.go index 6fde842dde16..5dcf5abbc845 100644 --- a/control-plane-operator/main.go +++ b/control-plane-operator/main.go @@ -533,7 +533,7 @@ func NewStartCommand() *cobra.Command { CertRotationScale: certRotationScale, EnableCVOManagementClusterMetricsAccess: enableCVOManagementClusterMetricsAccess, ImageMetadataProvider: imageMetaDataProvider, - }).SetupWithManager(mgr, upsert.New(enableCIDebugOutput).CreateOrUpdate, hcp); err != nil { + }).SetupWithManager(mgr, upsert.New(enableCIDebugOutput).CreateOrUpdate, hcp, mgr.GetLogger().WithName("hostedcontrolplane")); err != nil { setupLog.Error(err, "unable to create controller", "controller", "hosted-control-plane") os.Exit(1) } diff --git a/hypershift-operator/controllers/auditlogpersistence/configmap_webhook.go b/hypershift-operator/controllers/auditlogpersistence/configmap_webhook.go index 1e835a32011a..34dfde6ec3ca 100644 --- a/hypershift-operator/controllers/auditlogpersistence/configmap_webhook.go +++ b/hypershift-operator/controllers/auditlogpersistence/configmap_webhook.go @@ -14,6 +14,7 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" @@ -26,22 +27,22 @@ const ( ) type ConfigMapWebhookHandler struct { - log logr.Logger client client.Client decoder admission.Decoder } var _ admission.Handler = &ConfigMapWebhookHandler{} -func NewConfigMapWebhookHandler(log logr.Logger, c client.Client, decoder admission.Decoder) *ConfigMapWebhookHandler { +func NewConfigMapWebhookHandler(c client.Client, decoder admission.Decoder) *ConfigMapWebhookHandler { return &ConfigMapWebhookHandler{ - log: log.WithName("audit-log-persistence-configmap-webhook"), client: c, decoder: decoder, } } func (h *ConfigMapWebhookHandler) Handle(ctx context.Context, req admission.Request) admission.Response { + log := ctrl.LoggerFrom(ctx).WithName("audit-log-persistence-configmap-webhook") + // Only handle ConfigMap resources if req.Kind.Group != "" || req.Kind.Kind != "ConfigMap" { return admission.Allowed("") @@ -63,7 +64,7 @@ func (h *ConfigMapWebhookHandler) Handle(ctx context.Context, req admission.Requ if apierrors.IsNotFound(err) { return admission.Allowed("") } - h.log.Error(err, "Failed to get namespace", "namespace", req.Namespace) + log.Error(err, "Failed to get namespace", "namespace", req.Namespace) return admission.Errored(http.StatusInternalServerError, fmt.Errorf("failed to get namespace %s: %w", req.Namespace, err)) } @@ -74,7 +75,7 @@ func (h *ConfigMapWebhookHandler) Handle(ctx context.Context, req admission.Requ // Decode the ConfigMap configMap := &corev1.ConfigMap{} if err := h.decoder.Decode(req, configMap); err != nil { - h.log.Error(err, "Failed to decode ConfigMap") + log.Error(err, "Failed to decode ConfigMap") return admission.Errored(http.StatusBadRequest, err) } @@ -84,7 +85,7 @@ func (h *ConfigMapWebhookHandler) Handle(ctx context.Context, req admission.Requ if apierrors.IsNotFound(err) { return admission.Allowed("") } - h.log.Error(err, "Failed to get AuditLogPersistenceConfig") + log.Error(err, "Failed to get AuditLogPersistenceConfig") return admission.Errored(http.StatusInternalServerError, fmt.Errorf("failed to get AuditLogPersistenceConfig: %w", err)) } @@ -99,14 +100,14 @@ func (h *ConfigMapWebhookHandler) Handle(ctx context.Context, req admission.Requ // Mutate the ConfigMap mutated := configMap.DeepCopy() - if err := h.mutateConfigMap(mutated, spec); err != nil { - h.log.Error(err, "Failed to mutate ConfigMap") + if err := h.mutateConfigMap(mutated, spec, log); err != nil { + log.Error(err, "Failed to mutate ConfigMap") return admission.Errored(http.StatusInternalServerError, fmt.Errorf("failed to mutate ConfigMap: %w", err)) } mutatedRaw, err := json.Marshal(mutated) if err != nil { - h.log.Error(err, "Failed to marshal mutated ConfigMap") + log.Error(err, "Failed to marshal mutated ConfigMap") return admission.Errored(http.StatusInternalServerError, fmt.Errorf("failed to marshal mutated ConfigMap: %w", err)) } @@ -118,11 +119,11 @@ func (h *ConfigMapWebhookHandler) Handle(ctx context.Context, req admission.Requ if spec.AuditLog.MaxBackup != nil { maxBackupVal = *spec.AuditLog.MaxBackup } - h.log.Info("Successfully mutated ConfigMap for audit log persistence", "configmap", configMap.Name, "namespace", configMap.Namespace, "maxSize", maxSizeVal, "maxBackup", maxBackupVal) + log.Info("Successfully mutated ConfigMap for audit log persistence", "configmap", configMap.Name, "namespace", configMap.Namespace, "maxSize", maxSizeVal, "maxBackup", maxBackupVal) return admission.PatchResponseFromRaw(req.Object.Raw, mutatedRaw) } -func (h *ConfigMapWebhookHandler) mutateConfigMap(configMap *corev1.ConfigMap, spec *auditlogpersistencev1alpha1.AuditLogPersistenceConfigSpec) error { +func (h *ConfigMapWebhookHandler) mutateConfigMap(configMap *corev1.ConfigMap, spec *auditlogpersistencev1alpha1.AuditLogPersistenceConfigSpec, log logr.Logger) error { if configMap.Data == nil { configMap.Data = make(map[string]string) } @@ -135,14 +136,14 @@ func (h *ConfigMapWebhookHandler) mutateConfigMap(configMap *corev1.ConfigMap, s // Parse the JSON config into unstructured map var kasConfigMap map[string]interface{} if err := json.Unmarshal([]byte(configJSON), &kasConfigMap); err != nil { - h.log.Error(err, "Failed to unmarshal kube-apiserver config") + log.Error(err, "Failed to unmarshal kube-apiserver config") return fmt.Errorf("failed to unmarshal kube-apiserver config: %w", err) } // Ensure apiServerArguments exists apiServerArgs, exists, err := unstructured.NestedMap(kasConfigMap, "apiServerArguments") if err != nil { - h.log.Error(err, "Failed to get apiServerArguments") + log.Error(err, "Failed to get apiServerArguments") return fmt.Errorf("failed to get apiServerArguments: %w", err) } if !exists || apiServerArgs == nil { @@ -163,14 +164,14 @@ func (h *ConfigMapWebhookHandler) mutateConfigMap(configMap *corev1.ConfigMap, s // Set the updated apiServerArguments back if err := unstructured.SetNestedField(kasConfigMap, apiServerArgs, "apiServerArguments"); err != nil { - h.log.Error(err, "Failed to set apiServerArguments") + log.Error(err, "Failed to set apiServerArguments") return fmt.Errorf("failed to set apiServerArguments: %w", err) } // Serialize back to JSON updatedConfigJSON, err := json.Marshal(kasConfigMap) if err != nil { - h.log.Error(err, "Failed to marshal updated kube-apiserver config") + log.Error(err, "Failed to marshal updated kube-apiserver config") return fmt.Errorf("failed to marshal updated kube-apiserver config: %w", err) } diff --git a/hypershift-operator/controllers/auditlogpersistence/configmap_webhook_test.go b/hypershift-operator/controllers/auditlogpersistence/configmap_webhook_test.go index 0a35ab051281..96faa6069a6e 100644 --- a/hypershift-operator/controllers/auditlogpersistence/configmap_webhook_test.go +++ b/hypershift-operator/controllers/auditlogpersistence/configmap_webhook_test.go @@ -405,12 +405,11 @@ func TestMutateConfigMap(t *testing.T) { g := NewWithT(t) handler := &ConfigMapWebhookHandler{ - log: logr.Discard(), client: nil, decoder: nil, } - err := handler.mutateConfigMap(tt.configMap, &tt.config.Spec) + err := handler.mutateConfigMap(tt.configMap, &tt.config.Spec, logr.Discard()) if tt.expectedError { g.Expect(err).To(HaveOccurred()) diff --git a/hypershift-operator/controllers/auditlogpersistence/pod_webhook.go b/hypershift-operator/controllers/auditlogpersistence/pod_webhook.go index 0604e5c452fb..86156d268084 100644 --- a/hypershift-operator/controllers/auditlogpersistence/pod_webhook.go +++ b/hypershift-operator/controllers/auditlogpersistence/pod_webhook.go @@ -16,6 +16,7 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apiserver/pkg/storage/names" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" @@ -32,22 +33,22 @@ const ( ) type PodWebhookHandler struct { - log logr.Logger client client.Client decoder admission.Decoder } var _ admission.Handler = &PodWebhookHandler{} -func NewPodWebhookHandler(log logr.Logger, c client.Client, decoder admission.Decoder) *PodWebhookHandler { +func NewPodWebhookHandler(c client.Client, decoder admission.Decoder) *PodWebhookHandler { return &PodWebhookHandler{ - log: log.WithName("audit-log-persistence-pod-webhook"), client: c, decoder: decoder, } } func (h *PodWebhookHandler) Handle(ctx context.Context, req admission.Request) admission.Response { + log := ctrl.LoggerFrom(ctx).WithName("audit-log-persistence-pod-webhook") + // Only handle Pod resources if req.Kind.Group != "" || req.Kind.Kind != "Pod" { return admission.Allowed("") @@ -64,7 +65,7 @@ func (h *PodWebhookHandler) Handle(ctx context.Context, req admission.Request) a if apierrors.IsNotFound(err) { return admission.Allowed("") } - h.log.Error(err, "Failed to get namespace", "namespace", req.Namespace) + log.Error(err, "Failed to get namespace", "namespace", req.Namespace) return admission.Errored(http.StatusInternalServerError, fmt.Errorf("failed to get namespace %s: %w", req.Namespace, err)) } @@ -75,7 +76,7 @@ func (h *PodWebhookHandler) Handle(ctx context.Context, req admission.Request) a // Decode the pod first to check both name and generateName pod := &corev1.Pod{} if err := h.decoder.Decode(req, pod); err != nil { - h.log.Error(err, "Failed to decode pod") + log.Error(err, "Failed to decode pod") return admission.Errored(http.StatusBadRequest, err) } @@ -101,7 +102,7 @@ func (h *PodWebhookHandler) Handle(ctx context.Context, req admission.Request) a if apierrors.IsNotFound(err) { return admission.Allowed("") } - h.log.Error(err, "Failed to get AuditLogPersistenceConfig") + log.Error(err, "Failed to get AuditLogPersistenceConfig") return admission.Errored(http.StatusInternalServerError, fmt.Errorf("failed to get AuditLogPersistenceConfig: %w", err)) } @@ -116,28 +117,28 @@ func (h *PodWebhookHandler) Handle(ctx context.Context, req admission.Request) a // Mutate the pod mutated := pod.DeepCopy() - if err := h.mutatePod(ctx, mutated, spec); err != nil { - h.log.Error(err, "Failed to mutate pod for audit log persistence") + if err := h.mutatePod(ctx, mutated, spec, log); err != nil { + log.Error(err, "Failed to mutate pod for audit log persistence") return admission.Errored(http.StatusInternalServerError, fmt.Errorf("failed to mutate pod: %w", err)) } mutatedRaw, err := json.Marshal(mutated) if err != nil { - h.log.Error(err, "Failed to marshal mutated pod") + log.Error(err, "Failed to marshal mutated pod") return admission.Errored(http.StatusInternalServerError, fmt.Errorf("failed to marshal mutated pod: %w", err)) } - h.log.Info("Successfully mutated pod for audit log persistence", "pod", mutated.Name, "namespace", pod.Namespace, "pvc", pvcNamePrefix+mutated.Name) + log.Info("Successfully mutated pod for audit log persistence", "pod", mutated.Name, "namespace", pod.Namespace, "pvc", pvcNamePrefix+mutated.Name) return admission.PatchResponseFromRaw(req.Object.Raw, mutatedRaw) } -func (h *PodWebhookHandler) mutatePod(ctx context.Context, pod *corev1.Pod, spec *auditlogpersistencev1alpha1.AuditLogPersistenceConfigSpec) error { +func (h *PodWebhookHandler) mutatePod(ctx context.Context, pod *corev1.Pod, spec *auditlogpersistencev1alpha1.AuditLogPersistenceConfigSpec, log logr.Logger) error { // If pod has generateName but no name, generate a final name // This ensures we have a stable name for PVC creation // Use the same name generator that Kubernetes uses internally if pod.Name == "" && pod.GenerateName != "" { generatedName := names.SimpleNameGenerator.GenerateName(pod.GenerateName) - h.log.V(1).Info("Generating pod name from generateName", "generateName", pod.GenerateName, "generatedName", generatedName) + log.V(1).Info("Generating pod name from generateName", "generateName", pod.GenerateName, "generatedName", generatedName) pod.Name = generatedName pod.GenerateName = "" } @@ -195,19 +196,19 @@ func (h *PodWebhookHandler) mutatePod(ctx context.Context, pod *corev1.Pod, spec // Create or update the PVC if err := h.client.Create(ctx, pvc); err != nil { if !apierrors.IsAlreadyExists(err) { - h.log.Error(err, "Failed to create PVC", "pvcName", pvcName) + log.Error(err, "Failed to create PVC", "pvcName", pvcName) return fmt.Errorf("failed to create PVC %s: %w", pvcName, err) } // PVC already exists, update owner references if needed existingPVC := &corev1.PersistentVolumeClaim{} if err := h.client.Get(ctx, types.NamespacedName{Name: pvcName, Namespace: pod.Namespace}, existingPVC); err != nil { - h.log.Error(err, "Failed to get existing PVC", "pvcName", pvcName) + log.Error(err, "Failed to get existing PVC", "pvcName", pvcName) return fmt.Errorf("failed to get existing PVC %s: %w", pvcName, err) } if replicaSetOwner != nil && len(existingPVC.OwnerReferences) == 0 { existingPVC.OwnerReferences = pvc.OwnerReferences if err := h.client.Update(ctx, existingPVC); err != nil { - h.log.Error(err, "Failed to update PVC owner references", "pvcName", pvcName) + log.Error(err, "Failed to update PVC owner references", "pvcName", pvcName) return fmt.Errorf("failed to update PVC %s: %w", pvcName, err) } } diff --git a/hypershift-operator/controllers/auditlogpersistence/pod_webhook_test.go b/hypershift-operator/controllers/auditlogpersistence/pod_webhook_test.go index 10ae914cbdff..57486320f40a 100644 --- a/hypershift-operator/controllers/auditlogpersistence/pod_webhook_test.go +++ b/hypershift-operator/controllers/auditlogpersistence/pod_webhook_test.go @@ -541,14 +541,13 @@ func TestMutatePod(t *testing.T) { Build() handler := &PodWebhookHandler{ - log: logr.Discard(), client: fakeClient, } // Create a copy of the pod for mutation podCopy := tt.pod.DeepCopy() - err := handler.mutatePod(context.Background(), podCopy, &tt.config.Spec) + err := handler.mutatePod(context.Background(), podCopy, &tt.config.Spec, logr.Discard()) if tt.expectedError { g.Expect(err).To(HaveOccurred()) diff --git a/hypershift-operator/controllers/auditlogpersistence/snapshot_controller.go b/hypershift-operator/controllers/auditlogpersistence/snapshot_controller.go index 7d0d6a388326..753025afb0dc 100644 --- a/hypershift-operator/controllers/auditlogpersistence/snapshot_controller.go +++ b/hypershift-operator/controllers/auditlogpersistence/snapshot_controller.go @@ -41,7 +41,6 @@ const ( type SnapshotReconciler struct { client client.Client - log logr.Logger } // SetupSnapshotController sets up the snapshot controller that watches Pods and creates @@ -49,7 +48,6 @@ type SnapshotReconciler struct { func SetupSnapshotController(mgr ctrl.Manager) error { reconciler := &SnapshotReconciler{ client: mgr.GetClient(), - log: mgr.GetLogger().WithName(snapshotControllerName), } err := ctrl.NewControllerManagedBy(mgr). @@ -139,7 +137,7 @@ func (r *SnapshotReconciler) checkSnapshotInterval(ctx context.Context, pod *cor } func (r *SnapshotReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { - log := r.log.WithValues("pod", req.NamespacedName) + log := ctrl.LoggerFrom(ctx) pod := &corev1.Pod{} if err := r.client.Get(ctx, req.NamespacedName, pod); err != nil { diff --git a/hypershift-operator/controllers/auditlogpersistence/snapshot_controller_test.go b/hypershift-operator/controllers/auditlogpersistence/snapshot_controller_test.go index 35835b5e73a1..293190504bf7 100644 --- a/hypershift-operator/controllers/auditlogpersistence/snapshot_controller_test.go +++ b/hypershift-operator/controllers/auditlogpersistence/snapshot_controller_test.go @@ -788,7 +788,6 @@ func TestSnapshotReconciler_Reconcile(t *testing.T) { reconciler := &SnapshotReconciler{ client: fakeClient, - log: logr.Discard(), } req := reconcile.Request{ @@ -1084,7 +1083,6 @@ func TestSnapshotReconciler_manageRetention(t *testing.T) { reconciler := &SnapshotReconciler{ client: fakeClient, - log: logr.Discard(), } err := reconciler.manageRetention(context.Background(), tt.pod, tt.pvc, &tt.config.Spec) @@ -1360,7 +1358,6 @@ func TestGetLastObservedRestartCount(t *testing.T) { reconciler := &SnapshotReconciler{ client: fakeClient, - log: logr.Discard(), } // Save original annotation value before calling function to avoid checking mutated map @@ -1455,7 +1452,6 @@ func TestCheckSnapshotInterval(t *testing.T) { reconciler := &SnapshotReconciler{ client: fakeClient, - log: logr.Discard(), } spec := &auditlogpersistencev1alpha1.AuditLogPersistenceConfigSpec{ @@ -1540,7 +1536,6 @@ func TestGetSnapshotConfig(t *testing.T) { reconciler := &SnapshotReconciler{ client: fakeClient, - log: logr.Discard(), } spec, err := reconciler.getSnapshotConfig(context.Background()) @@ -1648,7 +1643,6 @@ func TestSnapshotReconciler_createSnapshot(t *testing.T) { reconciler := &SnapshotReconciler{ client: fakeClient, - log: logr.Discard(), } err := reconciler.createSnapshot(context.Background(), tt.pod, tt.pvc, &tt.config.Spec) diff --git a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go index 0f497ad5875d..08ad24ed9a40 100644 --- a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go +++ b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go @@ -158,6 +158,8 @@ var ( CAPIComponents = []string{capimanagerv2.ComponentName, capiproviderv2.ComponentName} ) +const ControllerName = "hostedcluster" + // HostedClusterReconciler reconciles a HostedCluster object type HostedClusterReconciler struct { client.Client @@ -246,6 +248,7 @@ func (r *HostedClusterReconciler) SetupWithManager(mgr ctrl.Manager, createOrUpd // namespaces, the events are filtered to enqueue only those resources which // are annotated as being associated with a hostedcluster (using an annotation). bldr := ctrl.NewControllerManagedBy(mgr). + Named(ControllerName). For(&hyperv1.HostedCluster{}, builder.WithPredicates(hyperutil.PredicatesForHostedClusterAnnotationScoping(mgr.GetClient()))). WithOptions(controller.Options{ RateLimiter: workqueue.NewTypedItemExponentialFailureRateLimiter[reconcile.Request](1*time.Second, 10*time.Second), diff --git a/hypershift-operator/controllers/nodepool/nodepool_controller.go b/hypershift-operator/controllers/nodepool/nodepool_controller.go index aab79922eefd..9e43fcabfcfa 100644 --- a/hypershift-operator/controllers/nodepool/nodepool_controller.go +++ b/hypershift-operator/controllers/nodepool/nodepool_controller.go @@ -98,6 +98,8 @@ const ( NTOMirroredConfigLabel = "hypershift.openshift.io/mirrored-config" ) +const ControllerName = "nodepool" + type NodePoolReconciler struct { client.Client recorder record.EventRecorder @@ -134,6 +136,7 @@ var capiRelatedNodePoolManagedResourcesToWatch = []client.Object{ func (r *NodePoolReconciler) SetupWithManager(mgr ctrl.Manager) error { bldr := ctrl.NewControllerManagedBy(mgr). + Named(ControllerName). For(&hyperv1.NodePool{}, builder.WithPredicates(supportutil.PredicatesForHostedClusterAnnotationScoping(mgr.GetClient()))). // We want to reconcile when the HostedCluster IgnitionEndpoint is available. Watches(&hyperv1.HostedCluster{}, handler.EnqueueRequestsFromMapFunc(r.enqueueNodePoolsForHostedCluster), builder.WithPredicates(supportutil.PredicatesForHostedClusterAnnotationScoping(mgr.GetClient()))). @@ -155,6 +158,7 @@ func (r *NodePoolReconciler) SetupWithManager(mgr ctrl.Manager) error { } if err := ctrl.NewControllerManagedBy(mgr). + Named("secretjanitor"). For(&corev1.Secret{}, builder.WithPredicates(supportutil.PredicatesForHostedClusterAnnotationScoping(mgr.GetClient()))). WithOptions(controller.Options{ RateLimiter: workqueue.NewTypedItemExponentialFailureRateLimiter[reconcile.Request](1*time.Second, 10*time.Second), diff --git a/hypershift-operator/controllers/platform/aws/controller.go b/hypershift-operator/controllers/platform/aws/controller.go index 80e82a6576f7..e478f80f1d4c 100644 --- a/hypershift-operator/controllers/platform/aws/controller.go +++ b/hypershift-operator/controllers/platform/aws/controller.go @@ -55,6 +55,8 @@ const ( lbNotActiveRequeueDuration = 20 * time.Second ) +const ControllerName = "awsendpointservice" + // AWSEndpointServiceReconciler watches HC/NodePools/awsEndpointService and reconcile the awsEndpointService // CRs existing for the KubeAPIServerPrivateService and the PrivateRouterService. // It creates the endpoint service in AWS and keeps the SubnetIDs up to date so NodePools are able to attach to the service endpoint. @@ -96,6 +98,7 @@ func awsEndpointServicesByName(ns string) []reconcile.Request { func (r *AWSEndpointServiceReconciler) SetupWithManager(mgr ctrl.Manager) error { _, err := ctrl.NewControllerManagedBy(mgr). + Named(ControllerName). For(&hyperv1.AWSEndpointService{}). Watches(&hyperv1.NodePool{}, handler.Funcs{ CreateFunc: r.enqueueOnNodePoolCreate(mgr), diff --git a/hypershift-operator/controllers/platform/azure/controller.go b/hypershift-operator/controllers/platform/azure/controller.go index abe7b4f1c77f..8bb3db6ae85e 100644 --- a/hypershift-operator/controllers/platform/azure/controller.go +++ b/hypershift-operator/controllers/platform/azure/controller.go @@ -107,6 +107,8 @@ type SubnetsAPI interface { NewListPager(resourceGroupName string, virtualNetworkName string, options *armnetwork.SubnetsClientListOptions) *azruntime.Pager[armnetwork.SubnetsClientListResponse] } +const ControllerName = "azureprivatelinkservice" + // AzurePrivateLinkServiceController reconciles AzurePrivateLinkService resources. // It watches AzurePrivateLinkService CRDs across all namespaces and manages // the lifecycle of Azure Private Link Service resources. @@ -121,6 +123,7 @@ type AzurePrivateLinkServiceController struct { // SetupWithManager sets up the controller with the Manager. func (r *AzurePrivateLinkServiceController) SetupWithManager(mgr ctrl.Manager) error { _, err := ctrl.NewControllerManagedBy(mgr). + Named(ControllerName). For(&hyperv1.AzurePrivateLinkService{}). WithOptions(controller.Options{ RateLimiter: workqueue.NewTypedItemExponentialFailureRateLimiter[reconcile.Request](3*time.Second, 30*time.Second), diff --git a/hypershift-operator/controllers/platform/gcp/privateserviceconnect_controller.go b/hypershift-operator/controllers/platform/gcp/privateserviceconnect_controller.go index 6c32278d9eae..a925a0a36f2c 100644 --- a/hypershift-operator/controllers/platform/gcp/privateserviceconnect_controller.go +++ b/hypershift-operator/controllers/platform/gcp/privateserviceconnect_controller.go @@ -40,6 +40,8 @@ const ( // +kubebuilder:rbac:groups=hypershift.openshift.io,resources=gcpprivateserviceconnects/status,verbs=get;update;patch // +kubebuilder:rbac:groups=hypershift.openshift.io,resources=hostedclusters,verbs=get;list;watch +const ControllerName = "gcpprivateserviceconnect" + // ComputeClient abstracts the GCP Compute API calls used by the PSC controller. // Using an interface instead of *compute.Service enables unit testing with fakes. type ComputeClient interface { @@ -99,11 +101,10 @@ type GCPPrivateServiceConnectReconciler struct { GcpClient ComputeClient ProjectID string Region string - Log logr.Logger } // SetupWithManager sets up the controller with the Manager. -func (r *GCPPrivateServiceConnectReconciler) SetupWithManager(mgr ctrl.Manager) error { +func (r *GCPPrivateServiceConnectReconciler) SetupWithManager(mgr ctrl.Manager, logger logr.Logger) error { // Initialize GCP Compute Service client gcpComputeService, err := InitGCPComputeService(context.Background()) if err != nil { @@ -125,9 +126,10 @@ func (r *GCPPrivateServiceConnectReconciler) SetupWithManager(mgr ctrl.Manager) } r.Region = region - r.Log.Info("Initialized GCP platform information", "projectID", r.ProjectID, "region", r.Region) + logger.Info("Initialized GCP platform information", "projectID", r.ProjectID, "region", r.Region) return ctrl.NewControllerManagedBy(mgr). + Named(ControllerName). For(&hyperv1.GCPPrivateServiceConnect{}). // Note: Add HostedCluster watching if needed for network configuration changes // Watches(&source.Kind{Type: &hyperv1.HostedCluster{}}, @@ -137,7 +139,7 @@ func (r *GCPPrivateServiceConnectReconciler) SetupWithManager(mgr ctrl.Manager) // Reconcile reconciles GCPPrivateServiceConnect resources func (r *GCPPrivateServiceConnectReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { - log := r.Log.WithValues("gcpprivateserviceconnect", req.NamespacedName) + log := ctrl.LoggerFrom(ctx) // 1. Fetch GCPPrivateServiceConnect CR obj := &hyperv1.GCPPrivateServiceConnect{} @@ -240,7 +242,7 @@ func (r *GCPPrivateServiceConnectReconciler) reconcileGCPPrivateServiceConnectSp // lookupForwardingRule finds the ForwardingRule for the LoadBalancer IP and returns the full object. // Returns nil (no error) when the ILB is not yet provisioned. func (r *GCPPrivateServiceConnectReconciler) lookupForwardingRule(ctx context.Context, gcpPSC *hyperv1.GCPPrivateServiceConnect) (*compute.ForwardingRule, error) { - log := r.Log.WithValues("gcpprivateserviceconnect", gcpPSC.Name, "loadBalancerIP", gcpPSC.Spec.LoadBalancerIP) + log := ctrl.LoggerFrom(ctx).WithValues("loadBalancerIP", gcpPSC.Spec.LoadBalancerIP) // Use AIP-160 filter syntax for exact string matching filter := fmt.Sprintf(`IPAddress = "%s"`, gcpPSC.Spec.LoadBalancerIP) @@ -296,7 +298,7 @@ func (r *GCPPrivateServiceConnectReconciler) isSubnetInUse(ctx context.Context, // networkURL is the full GCP network URL from the forwarding rule (e.g. // "https://www.googleapis.com/compute/v1/projects/…/global/networks/my-vpc"). func (r *GCPPrivateServiceConnectReconciler) discoverNATSubnet(ctx context.Context, gcpPSC *hyperv1.GCPPrivateServiceConnect, networkURL string) (string, error) { - log := r.Log.WithValues("gcpprivateserviceconnect", gcpPSC.Name) + log := ctrl.LoggerFrom(ctx) // Filter server-side to only subnets with PSC purpose in the management cluster's VPC, // preventing cross-VPC selection when multiple management clusters share a GCP project. @@ -336,7 +338,7 @@ func (r *GCPPrivateServiceConnectReconciler) discoverNATSubnet(ctx context.Conte // reconcileServiceAttachment manages Service Attachment lifecycle func (r *GCPPrivateServiceConnectReconciler) reconcileServiceAttachment(ctx context.Context, gcpPSC *hyperv1.GCPPrivateServiceConnect, hc *hyperv1.HostedCluster) (ctrl.Result, error) { - log := r.Log.WithValues("gcpprivateserviceconnect", gcpPSC.Name) + log := ctrl.LoggerFrom(ctx) // 1. Construct unique Service Attachment name using cluster ID serviceAttachmentName := r.constructServiceAttachmentName(hc) @@ -484,7 +486,7 @@ func (r *GCPPrivateServiceConnectReconciler) updateStatusFromServiceAttachment(c // delete handles deletion of GCPPrivateServiceConnect resources and returns completion status func (r *GCPPrivateServiceConnectReconciler) delete(ctx context.Context, gcpPSC *hyperv1.GCPPrivateServiceConnect) (bool, error) { - log := r.Log.WithValues("gcpprivateserviceconnect", gcpPSC.Name) + log := ctrl.LoggerFrom(ctx) // Use Service Attachment name from status (set during creation) serviceAttachmentName := gcpPSC.Status.ServiceAttachmentName @@ -523,7 +525,7 @@ func (r *GCPPrivateServiceConnectReconciler) delete(ctx context.Context, gcpPSC // handleGCPError handles GCP API errors with appropriate retry logic func (r *GCPPrivateServiceConnectReconciler) handleGCPError(ctx context.Context, gcpPSC *hyperv1.GCPPrivateServiceConnect, reason string, err error) (ctrl.Result, error) { //nolint:unparam // error return kept for API consistency - log := r.Log.WithValues("gcpprivateserviceconnect", gcpPSC.Name) + log := ctrl.LoggerFrom(ctx) // Extract GCP error details var requeueAfter time.Duration diff --git a/hypershift-operator/controllers/platform/gcp/privateserviceconnect_controller_test.go b/hypershift-operator/controllers/platform/gcp/privateserviceconnect_controller_test.go index 8aa7d4e012b3..3163489883df 100644 --- a/hypershift-operator/controllers/platform/gcp/privateserviceconnect_controller_test.go +++ b/hypershift-operator/controllers/platform/gcp/privateserviceconnect_controller_test.go @@ -152,11 +152,10 @@ func TestReconcileGCPPrivateServiceConnectSpec(t *testing.T) { CreateOrUpdateProvider: upsert.New(false), ProjectID: "test-project", Region: "us-central1", - Log: testr.New(t), } // Test with pre-populated spec fields to avoid GCP API calls - err := r.reconcileGCPPrivateServiceConnectSpec(context.Background(), gcpPSC, hc) + err := r.reconcileGCPPrivateServiceConnectSpec(ctrl.LoggerInto(context.Background(), testr.New(t)), gcpPSC, hc) // Since ForwardingRuleName and NATSubnet are already set, this should succeed if err != nil { @@ -177,7 +176,6 @@ func TestReconcile_NotFound(t *testing.T) { r := &GCPPrivateServiceConnectReconciler{ Client: client, - Log: testr.New(t), } req := reconcile.Request{ @@ -187,7 +185,7 @@ func TestReconcile_NotFound(t *testing.T) { }, } - result, err := r.Reconcile(context.Background(), req) + result, err := r.Reconcile(ctrl.LoggerInto(context.Background(), testr.New(t)), req) if err != nil { t.Errorf("unexpected error: %v", err) @@ -252,7 +250,6 @@ func TestReconcile_PausedUntil(t *testing.T) { CreateOrUpdateProvider: upsert.New(false), ProjectID: "test-project", Region: "us-central1", - Log: testr.New(t), } req := reconcile.Request{ @@ -262,7 +259,7 @@ func TestReconcile_PausedUntil(t *testing.T) { }, } - result, err := r.Reconcile(context.Background(), req) + result, err := r.Reconcile(ctrl.LoggerInto(context.Background(), testr.New(t)), req) if err != nil { t.Errorf("unexpected error: %v", err) diff --git a/hypershift-operator/controllers/sharedingress/sharedingress_controller.go b/hypershift-operator/controllers/sharedingress/sharedingress_controller.go index 9f78b53984e2..5345f4bda775 100644 --- a/hypershift-operator/controllers/sharedingress/sharedingress_controller.go +++ b/hypershift-operator/controllers/sharedingress/sharedingress_controller.go @@ -8,7 +8,6 @@ import ( hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" assets "github.com/openshift/hypershift/cmd/install/assets" - "github.com/openshift/hypershift/cmd/log" "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/common" "github.com/openshift/hypershift/support/azureutil" "github.com/openshift/hypershift/support/capabilities" @@ -138,6 +137,7 @@ func (r *SharedIngressReconciler) SetupWithManager(mgr ctrl.Manager, createOrUpd } func (r *SharedIngressReconciler) Reconcile(ctx context.Context, _ ctrl.Request) (ctrl.Result, error) { + log := ctrl.LoggerFrom(ctx) namespace := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: RouterNamespace}} if _, err := r.createOrUpdate(ctx, r.Client, namespace, func() error { if namespace.Labels == nil { @@ -153,7 +153,7 @@ func (r *SharedIngressReconciler) Reconcile(ctx context.Context, _ ctrl.Request) src := &corev1.Secret{} if err := r.Client.Get(ctx, client.ObjectKey{Namespace: r.Namespace, Name: assets.PullSecretName}, src); err != nil { if errors.IsNotFound(err) { - log.Log.Info(fmt.Sprintf("pull secret was not found in %s namespace, will not create pullsecret for sharedingress", r.Namespace)) + log.Info(fmt.Sprintf("pull secret was not found in %s namespace, will not create pullsecret for sharedingress", r.Namespace)) } else { return ctrl.Result{}, fmt.Errorf("failed to get pull secret %s: %w", src, err) } @@ -185,6 +185,7 @@ func (r *SharedIngressReconciler) Reconcile(ctx context.Context, _ ctrl.Request) } func (r *SharedIngressReconciler) reconcileRouter(ctx context.Context, pullSecretPresent bool) error { + log := ctrl.LoggerFrom(ctx) if err := r.reconcileDefaultServiceAccount(ctx, pullSecretPresent); err != nil { return fmt.Errorf("failed to reconcile default service account: %w", err) } @@ -246,7 +247,7 @@ func (r *SharedIngressReconciler) reconcileRouter(ctx context.Context, pullSecre }); err != nil { return fmt.Errorf("failed to reconcile etcd pdb: %w", err) } else { - log.Log.Info("reconciled etcd pdb", "result", result) + log.Info("reconciled etcd pdb", "result", result) } // Reconcile KAS Network Policy @@ -265,7 +266,7 @@ func (r *SharedIngressReconciler) reconcileRouter(ctx context.Context, pullSecre }); err != nil { return fmt.Errorf("failed to reconcile router network policy: %w", err) } else { - log.Log.Info("reconciled router network policy", "result", result) + log.Info("reconciled router network policy", "result", result) } return nil diff --git a/hypershift-operator/controllers/supportedversion/reconciler.go b/hypershift-operator/controllers/supportedversion/reconciler.go index a006ce99da75..86c4e939e0cb 100644 --- a/hypershift-operator/controllers/supportedversion/reconciler.go +++ b/hypershift-operator/controllers/supportedversion/reconciler.go @@ -23,6 +23,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/source" ) +const ControllerName = "supportedversion" + type Reconciler struct { client.Client upsert.CreateOrUpdateProvider @@ -41,6 +43,7 @@ func (r *Reconciler) SetupWithManager(mgr manager.Manager) error { // Afterwards, the controller syncs on the ConfigMap. initialSync := make(chan event.GenericEvent) err := ctrl.NewControllerManagedBy(mgr). + Named(ControllerName). For(&corev1.ConfigMap{}, builder.WithPredicates(predicate.NewPredicateFuncs(r.selectSupportedVersionsConfigMap))). WatchesRawSource(source.Channel(initialSync, &handler.EnqueueRequestForObject{})). Complete(r) diff --git a/hypershift-operator/controllers/uwmtelemetry/uwm_telemetry.go b/hypershift-operator/controllers/uwmtelemetry/uwm_telemetry.go index 53e68f6d552c..86a05e1d9339 100644 --- a/hypershift-operator/controllers/uwmtelemetry/uwm_telemetry.go +++ b/hypershift-operator/controllers/uwmtelemetry/uwm_telemetry.go @@ -41,6 +41,8 @@ const ( telemetryRemoteWriteURL = "https://infogw.api.openshift.com/metrics/v1/receive" ) +const ControllerName = "uwmtelemetry" + type Reconciler struct { client.Client upsert.CreateOrUpdateProvider @@ -52,6 +54,7 @@ type Reconciler struct { func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error { // Reconcile on the HyperShift operator deployment and watch monitoring namespaces _, err := ctrl.NewControllerManagedBy(mgr). + Named(ControllerName). For(&appsv1.Deployment{}, builder.WithPredicates(predicateForNamespacedName(manifests.OperatorDeployment(r.Namespace)))). Watches(&corev1.Namespace{}, handler.EnqueueRequestsFromMapFunc(mapRequestTo(manifests.OperatorDeployment(r.Namespace))), builder.WithPredicates(predicateForNames(monitoring.MonitoringNamespace().Name, monitoring.UWMNamespace().Name))). diff --git a/hypershift-operator/main.go b/hypershift-operator/main.go index 00362ae3a634..02e217b45998 100644 --- a/hypershift-operator/main.go +++ b/hypershift-operator/main.go @@ -726,8 +726,7 @@ func setupPlatformControllers(mgr ctrl.Manager, opts *StartOptions, mgmtClusterC if err := (&gcp.GCPPrivateServiceConnectReconciler{ Client: mgr.GetClient(), CreateOrUpdateProvider: createOrUpdate, - Log: ctrl.Log.WithName("controllers").WithName("GCPPrivateServiceConnect"), - }).SetupWithManager(mgr); err != nil { + }).SetupWithManager(mgr, mgr.GetLogger().WithName(gcp.ControllerName)); err != nil { return fmt.Errorf("unable to create GCPPrivateServiceConnect controller: %w", err) } case hyperv1.AzurePlatform: @@ -1004,7 +1003,6 @@ func setupAuditLogPersistence(mgr ctrl.Manager, opts *StartOptions, log logr.Log hookServer := mgr.GetWebhookServer() hookServer.Register("/mutate-kas-audit-logs", &webhook.Admission{ Handler: auditlogpersistence.NewPodWebhookHandler( - mgr.GetLogger(), mgr.GetClient(), admission.NewDecoder(mgr.GetScheme()), ), @@ -1012,7 +1010,6 @@ func setupAuditLogPersistence(mgr ctrl.Manager, opts *StartOptions, log logr.Log hookServer.Register("/mutate-kas-audit-log-config", &webhook.Admission{ Handler: auditlogpersistence.NewConfigMapWebhookHandler( - mgr.GetLogger(), mgr.GetClient(), admission.NewDecoder(mgr.GetScheme()), ), diff --git a/product-cli/cmd/cluster/agent/destroy.go b/product-cli/cmd/cluster/agent/destroy.go index 63f826bd80ed..2a9bd11c83ab 100644 --- a/product-cli/cmd/cluster/agent/destroy.go +++ b/product-cli/cmd/cluster/agent/destroy.go @@ -3,7 +3,7 @@ package agent import ( "github.com/openshift/hypershift/cmd/cluster/agent" "github.com/openshift/hypershift/cmd/cluster/core" - "github.com/openshift/hypershift/cmd/log" + cmdutil "github.com/openshift/hypershift/cmd/util" "github.com/spf13/cobra" ) @@ -17,7 +17,7 @@ func NewDestroyCommand(opts *core.DestroyOptions) *cobra.Command { cmd.RunE = func(cmd *cobra.Command, args []string) error { if err := agent.DestroyCluster(cmd.Context(), opts); err != nil { - log.Log.Error(err, "Failed to destroy cluster") + cmdutil.NewLogger().Error(err, "Failed to destroy cluster") return err } diff --git a/product-cli/cmd/cluster/aws/destroy.go b/product-cli/cmd/cluster/aws/destroy.go index db86dbe04546..18a4a482bb80 100644 --- a/product-cli/cmd/cluster/aws/destroy.go +++ b/product-cli/cmd/cluster/aws/destroy.go @@ -3,7 +3,7 @@ package aws import ( hypershiftaws "github.com/openshift/hypershift/cmd/cluster/aws" "github.com/openshift/hypershift/cmd/cluster/core" - "github.com/openshift/hypershift/cmd/log" + cmdutil "github.com/openshift/hypershift/cmd/util" "github.com/spf13/cobra" ) @@ -36,7 +36,7 @@ func NewDestroyCommand(opts *core.DestroyOptions) *cobra.Command { } if err = hypershiftaws.DestroyCluster(cmd.Context(), opts); err != nil { - log.Log.Error(err, "Failed to destroy cluster") + cmdutil.NewLogger().Error(err, "Failed to destroy cluster") return err } diff --git a/product-cli/cmd/cluster/cluster.go b/product-cli/cmd/cluster/cluster.go index f8625cc663ae..982725ca55b0 100644 --- a/product-cli/cmd/cluster/cluster.go +++ b/product-cli/cmd/cluster/cluster.go @@ -5,8 +5,7 @@ import ( "github.com/openshift/hypershift/api/hypershift/v1beta1" "github.com/openshift/hypershift/cmd/cluster/core" - "github.com/openshift/hypershift/cmd/log" - "github.com/openshift/hypershift/cmd/util" + cmdutil "github.com/openshift/hypershift/cmd/util" "github.com/openshift/hypershift/product-cli/cmd/cluster/agent" "github.com/openshift/hypershift/product-cli/cmd/cluster/aws" "github.com/openshift/hypershift/product-cli/cmd/cluster/azure" @@ -44,7 +43,7 @@ func NewDestroyCommands() *cobra.Command { opts := &core.DestroyOptions{ ClusterGracePeriod: 10 * time.Minute, DestroyCloudResources: true, - Log: log.Log, + Log: cmdutil.NewLogger(), Name: "", Namespace: "clusters", } @@ -58,7 +57,7 @@ func NewDestroyCommands() *cobra.Command { cmd.PersistentFlags().DurationVar(&opts.ClusterGracePeriod, "cluster-grace-period", opts.ClusterGracePeriod, "Period of time to wait for the HostedCluster to be deleted before forcibly destroying its infrastructure.") cmd.PersistentFlags().BoolVar(&opts.DestroyCloudResources, "destroy-cloud-resources", opts.DestroyCloudResources, "If true, cloud resources, such as load balancers and persistent storage disks, created by the HostedCluster during its lifetime are removed.") cmd.PersistentFlags().StringVar(&opts.InfraID, "infra-id", opts.InfraID, "The HostedCluster's infrastructure ID. This is inferred from the HostedCluster by default.") - cmd.PersistentFlags().StringVar(&opts.Kubeconfig, "kubeconfig", opts.Kubeconfig, util.KubeconfigFlagHelp) + cmd.PersistentFlags().StringVar(&opts.Kubeconfig, "kubeconfig", opts.Kubeconfig, cmdutil.KubeconfigFlagHelp) cmd.PersistentFlags().StringVar(&opts.Name, "name", opts.Name, "The HostedCluster's name.") cmd.PersistentFlags().StringVar(&opts.Namespace, "namespace", opts.Namespace, "The HostedCluster's namespace name.") diff --git a/product-cli/cmd/cluster/kubevirt/destroy.go b/product-cli/cmd/cluster/kubevirt/destroy.go index 85a8d2338193..cd4be5d34cbf 100644 --- a/product-cli/cmd/cluster/kubevirt/destroy.go +++ b/product-cli/cmd/cluster/kubevirt/destroy.go @@ -3,7 +3,7 @@ package kubevirt import ( "github.com/openshift/hypershift/cmd/cluster/core" "github.com/openshift/hypershift/cmd/cluster/none" - "github.com/openshift/hypershift/cmd/log" + cmdutil "github.com/openshift/hypershift/cmd/util" "github.com/spf13/cobra" ) @@ -17,7 +17,7 @@ func NewDestroyCommand(opts *core.DestroyOptions) *cobra.Command { cmd.RunE = func(cmd *cobra.Command, args []string) error { if err := none.DestroyCluster(cmd.Context(), opts); err != nil { - log.Log.Error(err, "Failed to destroy cluster") + cmdutil.NewLogger().Error(err, "Failed to destroy cluster") return err } return nil diff --git a/product-cli/cmd/cluster/openstack/destroy.go b/product-cli/cmd/cluster/openstack/destroy.go index 48c47890fecc..f40e1b6044b6 100644 --- a/product-cli/cmd/cluster/openstack/destroy.go +++ b/product-cli/cmd/cluster/openstack/destroy.go @@ -8,7 +8,7 @@ import ( "github.com/openshift/hypershift/cmd/cluster/core" "github.com/openshift/hypershift/cmd/cluster/openstack" - "github.com/openshift/hypershift/cmd/log" + cmdutil "github.com/openshift/hypershift/cmd/util" "github.com/spf13/cobra" ) @@ -20,7 +20,7 @@ func NewDestroyCommand(opts *core.DestroyOptions) *cobra.Command { SilenceUsage: true, } - logger := log.Log + logger := cmdutil.NewLogger() cmd.Run = func(cmd *cobra.Command, args []string) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() diff --git a/product-cli/cmd/iam/azure/create.go b/product-cli/cmd/iam/azure/create.go index 57b0aed44797..2c4c36010c75 100644 --- a/product-cli/cmd/iam/azure/create.go +++ b/product-cli/cmd/iam/azure/create.go @@ -2,7 +2,7 @@ package azure import ( hypershiftazure "github.com/openshift/hypershift/cmd/infra/azure" - "github.com/openshift/hypershift/cmd/log" + cmdutil "github.com/openshift/hypershift/cmd/util" "github.com/spf13/cobra" ) @@ -25,7 +25,7 @@ func NewCreateCommand() *cobra.Command { _ = cmd.MarkFlagRequired("oidc-issuer-url") _ = cmd.MarkFlagRequired("output-file") - l := log.Log + l := cmdutil.NewLogger() cmd.RunE = func(cmd *cobra.Command, args []string) error { if err := opts.Validate(); err != nil { return err diff --git a/product-cli/cmd/iam/azure/destroy.go b/product-cli/cmd/iam/azure/destroy.go index d97365845f62..48ded0c77d39 100644 --- a/product-cli/cmd/iam/azure/destroy.go +++ b/product-cli/cmd/iam/azure/destroy.go @@ -2,7 +2,7 @@ package azure import ( hypershiftazure "github.com/openshift/hypershift/cmd/infra/azure" - "github.com/openshift/hypershift/cmd/log" + cmdutil "github.com/openshift/hypershift/cmd/util" "github.com/spf13/cobra" ) @@ -24,7 +24,7 @@ func NewDestroyCommand() *cobra.Command { _ = cmd.MarkFlagRequired("azure-creds") _ = cmd.MarkFlagRequired("resource-group-name") - l := log.Log + l := cmdutil.NewLogger() cmd.RunE = func(cmd *cobra.Command, args []string) error { if err := opts.Validate(); err != nil { return err diff --git a/product-cli/cmd/infra/azure/create.go b/product-cli/cmd/infra/azure/create.go index ba99bc23ef6d..f02e1e7c8bec 100644 --- a/product-cli/cmd/infra/azure/create.go +++ b/product-cli/cmd/infra/azure/create.go @@ -2,7 +2,7 @@ package azure import ( hypershiftazure "github.com/openshift/hypershift/cmd/infra/azure" - "github.com/openshift/hypershift/cmd/log" + cmdutil "github.com/openshift/hypershift/cmd/util" "github.com/spf13/cobra" ) @@ -22,7 +22,7 @@ func NewCreateCommand() *cobra.Command { _ = cmd.MarkFlagRequired("azure-creds") _ = cmd.MarkFlagRequired("name") - l := log.Log + l := cmdutil.NewLogger() cmd.RunE = func(cmd *cobra.Command, args []string) error { if err := opts.Validate(); err != nil { return err diff --git a/product-cli/cmd/infra/azure/destroy.go b/product-cli/cmd/infra/azure/destroy.go index 86c4e315ef74..ba692b5918c6 100644 --- a/product-cli/cmd/infra/azure/destroy.go +++ b/product-cli/cmd/infra/azure/destroy.go @@ -2,7 +2,7 @@ package azure import ( hypershiftazure "github.com/openshift/hypershift/cmd/infra/azure" - "github.com/openshift/hypershift/cmd/log" + cmdutil "github.com/openshift/hypershift/cmd/util" "github.com/spf13/cobra" ) @@ -22,7 +22,7 @@ func NewDestroyCommand() *cobra.Command { _ = cmd.MarkFlagRequired("azure-creds") _ = cmd.MarkFlagRequired("name") - l := log.Log + l := cmdutil.NewLogger() cmd.RunE = func(cmd *cobra.Command, args []string) error { if err := opts.Validate(); err != nil { return err diff --git a/product-cli/cmd/nodepool/destroy.go b/product-cli/cmd/nodepool/destroy.go index d2767e2a4a17..3cffff14f4cc 100644 --- a/product-cli/cmd/nodepool/destroy.go +++ b/product-cli/cmd/nodepool/destroy.go @@ -5,8 +5,7 @@ import ( "fmt" hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" - "github.com/openshift/hypershift/cmd/log" - "github.com/openshift/hypershift/cmd/util" + cmdutil "github.com/openshift/hypershift/cmd/util" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -44,7 +43,7 @@ func NewDestroyCommand() *cobra.Command { } func (o *DestroyNodePoolOptions) Run(ctx context.Context) error { - client, err := util.GetClient() + client, err := cmdutil.GetClient() if err != nil { return err } @@ -59,12 +58,12 @@ func (o *DestroyNodePoolOptions) run(ctx context.Context, client crclient.Client if err := client.Delete(ctx, nodePool); err != nil { if apierrors.IsNotFound(err) { - log.Log.Info("NodePool already deleted or not found", "name", o.Name, "namespace", o.Namespace) + cmdutil.NewLogger().Info("NodePool already deleted or not found", "name", o.Name, "namespace", o.Namespace) return nil } return fmt.Errorf("failed to delete NodePool %s/%s: %w", o.Namespace, o.Name, err) } - log.Log.Info("NodePool deleted successfully", "name", o.Name, "namespace", o.Namespace) + cmdutil.NewLogger().Info("NodePool deleted successfully", "name", o.Name, "namespace", o.Namespace) return nil }