From 6a93ddbc8582b288a0dc6e08d6b99360e2478e46 Mon Sep 17 00:00:00 2001 From: Hadrien Patte Date: Mon, 8 Dec 2025 18:00:08 +0100 Subject: [PATCH 1/5] External-ccm: Fix go formatting Go source files are currently not properly formatted, making `make gofmt` fail. This PR addresses that. Signed-off-by: Hadrien Patte --- .../csioptions/csioptions.go | 114 +++++++++--------- .../nodedriver/nodedriver.go | 2 +- .../nodedriveroptions/nodecsioptions.go | 6 +- .../providers/oci/load_balancer_util_test.go | 2 +- pkg/csi-util/lustre_lnet_helper.go | 16 +-- pkg/csi-util/utils.go | 18 ++- pkg/csi-util/utils_test.go | 45 ++++--- pkg/csi/driver/fss_controller.go | 10 +- pkg/csi/driver/fss_controller_test.go | 6 +- pkg/csi/driver/lustre_node.go | 13 +- pkg/oci/client/block_storage.go | 2 +- pkg/oci/client/client_test.go | 4 +- pkg/oci/client/generic_load_balancer_types.go | 2 +- pkg/oci/client/volume_attachment_test.go | 17 ++- pkg/util/disk/iscsi.go | 4 +- pkg/util/disk/paravirtualized.go | 2 +- pkg/util/osinfo/osinfo.go | 9 +- pkg/util/signals/signal_posix.go | 1 + test/e2e/cloud-provider-oci/lustre_static.go | 18 +-- test/e2e/framework/framework.go | 50 ++++---- .../e2e/framework/volumesnapshotclass_util.go | 6 +- 21 files changed, 167 insertions(+), 180 deletions(-) diff --git a/cmd/oci-csi-controller-driver/csioptions/csioptions.go b/cmd/oci-csi-controller-driver/csioptions/csioptions.go index 6e478a19c1..0f0934f112 100644 --- a/cmd/oci-csi-controller-driver/csioptions/csioptions.go +++ b/cmd/oci-csi-controller-driver/csioptions/csioptions.go @@ -26,75 +26,73 @@ const ( fssAddressSuffix = "-fss.sock" fssVolumeNameAppendedPrefix = "-fss" CrossNamespaceVolumeDataSource = "CrossNamespaceVolumeDataSource" - VolumeAttributesClass = "VolumeAttributesClass" + VolumeAttributesClass = "VolumeAttributesClass" ) // CSIOptions structure which contains flag values type CSIOptions struct { - Master string - Kubeconfig string - CsiAddress string - Endpoint string - FssCsiAddress string - FssEndpoint string - VolumeNamePrefix string - FssVolumeNamePrefix string - VolumeNameUUIDLength int - ShowVersion bool - RetryIntervalStart time.Duration - RetryIntervalMax time.Duration - WorkerThreads uint - OperationTimeout time.Duration - EnableLeaderElection bool - LeaderElectionType string - LeaderElectionNamespace string - StrictTopology bool - Resync time.Duration - Timeout time.Duration - FeatureGates map[string]bool - FinalizerThreads uint - MetricsAddress string - MetricsPath string - ExtraCreateMetadata bool - ReconcileSync time.Duration - EnableResizer bool - GroupSnapshotNamePrefix string + Master string + Kubeconfig string + CsiAddress string + Endpoint string + FssCsiAddress string + FssEndpoint string + VolumeNamePrefix string + FssVolumeNamePrefix string + VolumeNameUUIDLength int + ShowVersion bool + RetryIntervalStart time.Duration + RetryIntervalMax time.Duration + WorkerThreads uint + OperationTimeout time.Duration + EnableLeaderElection bool + LeaderElectionType string + LeaderElectionNamespace string + StrictTopology bool + Resync time.Duration + Timeout time.Duration + FeatureGates map[string]bool + FinalizerThreads uint + MetricsAddress string + MetricsPath string + ExtraCreateMetadata bool + ReconcileSync time.Duration + EnableResizer bool + GroupSnapshotNamePrefix string GroupSnapshotNameUUIDLength int - } // NewCSIOptions initializes the flag func NewCSIOptions() *CSIOptions { csioptions := CSIOptions{ - Master: *flag.String("master", "", "kube master"), - Kubeconfig: *flag.String("kubeconfig", "", "cluster kube config"), - CsiAddress: *flag.String("csi-address", "/run/csi/socket", "Address of the CSI BV driver socket."), - Endpoint: *flag.String("csi-endpoint", "unix://tmp/csi.sock", "CSI BV endpoint"), - FssCsiAddress: *flag.String("fss-csi-address", "/run/fss/socket", "Address of the CSI FSS driver socket."), - FssEndpoint: *flag.String("fss-csi-endpoint", "unix://tmp/csi-fss.sock", "CSI FSS endpoint"), - VolumeNamePrefix: *flag.String("csi-volume-name-prefix", "pvc", "Prefix to apply to the name of a created volume."), - FssVolumeNamePrefix: *flag.String("fss-csi-volume-name-prefix", "pvc", "Prefix to apply to the name of a volume created for FSS."), - VolumeNameUUIDLength: *flag.Int("csi-volume-name-uuid-length", -1, "Truncates generated UUID of a created volume to this length. Defaults behavior is to NOT truncate."), - ShowVersion: *flag.Bool("csi-version", false, "Show version."), - RetryIntervalStart: *flag.Duration("csi-retry-interval-start", time.Second, "Initial retry interval of failed provisioning or deletion. It doubles with each failure, up to retry-interval-max."), - RetryIntervalMax: *flag.Duration("csi-retry-interval-max", 5*time.Minute, "Maximum retry interval of failed provisioning or deletion."), - WorkerThreads: *flag.Uint("csi-worker-threads", 100, "Number of provisioner worker threads, in other words nr. of simultaneous CSI calls."), - OperationTimeout: *flag.Duration("csi-op-timeout", 10*time.Second, "Timeout for waiting for creation or deletion of a volume"), - EnableLeaderElection: *flag.Bool("csi-enable-leader-election", false, "Enables leader election. If leader election is enabled, additional RBAC rules are required. Please refer to the Kubernetes CSI documentation for instructions on setting up these RBAC rules."), - LeaderElectionType: *flag.String("csi-leader-election-type", "endpoints", "the type of leader election, options are 'endpoints' (default) or 'leases' (strongly recommended). The 'endpoints' option is deprecated in favor of 'leases'."), - LeaderElectionNamespace: *flag.String("csi-leader-election-namespace", "", "Namespace where the leader election resource lives. Defaults to the pod namespace if not set."), - StrictTopology: *flag.Bool("csi-strict-topology", false, "Passes only selected node topology to CreateVolume Request, unlike default behavior of passing aggregated cluster topologies that match with topology keys of the selected node."), - Resync: *flag.Duration("csi-resync", 10*time.Minute, "Resync interval of the controller."), - Timeout: *flag.Duration("csi-timeout", 15*time.Second, "Timeout for waiting for attaching or detaching the volume."), - FinalizerThreads: *flag.Uint("cloning-protection-threads", 1, "Number of simultaniously running threads, handling cloning finalizer removal"), - MetricsAddress: *flag.String("metrics-address", "", "The TCP network address where the prometheus metrics endpoint will listen (example: `:8080`). The default is empty string, which means metrics endpoint is disabled."), - MetricsPath: *flag.String("metrics-path", "/metrics", "The HTTP path where prometheus metrics will be exposed. Default is `/metrics`."), - ExtraCreateMetadata: *flag.Bool("extra-create-metadata", false, "If set, add pv/pvc metadata to plugin create requests as parameters."), - ReconcileSync: *flag.Duration("reconcile-sync", 1*time.Minute, "Resync interval of the VolumeAttachment reconciler."), - EnableResizer: *flag.Bool("csi-bv-expansion-enabled", false, "Enables go routine csi-resizer."), - GroupSnapshotNamePrefix: *flag.String("groupsnapshot-name-prefix", "groupsnapshot", "Prefix to apply to the name of a created group snapshot"), + Master: *flag.String("master", "", "kube master"), + Kubeconfig: *flag.String("kubeconfig", "", "cluster kube config"), + CsiAddress: *flag.String("csi-address", "/run/csi/socket", "Address of the CSI BV driver socket."), + Endpoint: *flag.String("csi-endpoint", "unix://tmp/csi.sock", "CSI BV endpoint"), + FssCsiAddress: *flag.String("fss-csi-address", "/run/fss/socket", "Address of the CSI FSS driver socket."), + FssEndpoint: *flag.String("fss-csi-endpoint", "unix://tmp/csi-fss.sock", "CSI FSS endpoint"), + VolumeNamePrefix: *flag.String("csi-volume-name-prefix", "pvc", "Prefix to apply to the name of a created volume."), + FssVolumeNamePrefix: *flag.String("fss-csi-volume-name-prefix", "pvc", "Prefix to apply to the name of a volume created for FSS."), + VolumeNameUUIDLength: *flag.Int("csi-volume-name-uuid-length", -1, "Truncates generated UUID of a created volume to this length. Defaults behavior is to NOT truncate."), + ShowVersion: *flag.Bool("csi-version", false, "Show version."), + RetryIntervalStart: *flag.Duration("csi-retry-interval-start", time.Second, "Initial retry interval of failed provisioning or deletion. It doubles with each failure, up to retry-interval-max."), + RetryIntervalMax: *flag.Duration("csi-retry-interval-max", 5*time.Minute, "Maximum retry interval of failed provisioning or deletion."), + WorkerThreads: *flag.Uint("csi-worker-threads", 100, "Number of provisioner worker threads, in other words nr. of simultaneous CSI calls."), + OperationTimeout: *flag.Duration("csi-op-timeout", 10*time.Second, "Timeout for waiting for creation or deletion of a volume"), + EnableLeaderElection: *flag.Bool("csi-enable-leader-election", false, "Enables leader election. If leader election is enabled, additional RBAC rules are required. Please refer to the Kubernetes CSI documentation for instructions on setting up these RBAC rules."), + LeaderElectionType: *flag.String("csi-leader-election-type", "endpoints", "the type of leader election, options are 'endpoints' (default) or 'leases' (strongly recommended). The 'endpoints' option is deprecated in favor of 'leases'."), + LeaderElectionNamespace: *flag.String("csi-leader-election-namespace", "", "Namespace where the leader election resource lives. Defaults to the pod namespace if not set."), + StrictTopology: *flag.Bool("csi-strict-topology", false, "Passes only selected node topology to CreateVolume Request, unlike default behavior of passing aggregated cluster topologies that match with topology keys of the selected node."), + Resync: *flag.Duration("csi-resync", 10*time.Minute, "Resync interval of the controller."), + Timeout: *flag.Duration("csi-timeout", 15*time.Second, "Timeout for waiting for attaching or detaching the volume."), + FinalizerThreads: *flag.Uint("cloning-protection-threads", 1, "Number of simultaniously running threads, handling cloning finalizer removal"), + MetricsAddress: *flag.String("metrics-address", "", "The TCP network address where the prometheus metrics endpoint will listen (example: `:8080`). The default is empty string, which means metrics endpoint is disabled."), + MetricsPath: *flag.String("metrics-path", "/metrics", "The HTTP path where prometheus metrics will be exposed. Default is `/metrics`."), + ExtraCreateMetadata: *flag.Bool("extra-create-metadata", false, "If set, add pv/pvc metadata to plugin create requests as parameters."), + ReconcileSync: *flag.Duration("reconcile-sync", 1*time.Minute, "Resync interval of the VolumeAttachment reconciler."), + EnableResizer: *flag.Bool("csi-bv-expansion-enabled", false, "Enables go routine csi-resizer."), + GroupSnapshotNamePrefix: *flag.String("groupsnapshot-name-prefix", "groupsnapshot", "Prefix to apply to the name of a created group snapshot"), GroupSnapshotNameUUIDLength: *flag.Int("groupsnapshot-name-uuid-length", -1, "Length in characters for the generated uuid of a created group snapshot. Defaults behavior is to NOT truncate."), - } return &csioptions } diff --git a/cmd/oci-csi-node-driver/nodedriver/nodedriver.go b/cmd/oci-csi-node-driver/nodedriver/nodedriver.go index 65570cd88b..61a581fe91 100644 --- a/cmd/oci-csi-node-driver/nodedriver/nodedriver.go +++ b/cmd/oci-csi-node-driver/nodedriver/nodedriver.go @@ -21,7 +21,7 @@ import ( "go.uber.org/zap" ) -//RunNodeDriver main function to start node driver +// RunNodeDriver main function to start node driver func RunNodeDriver(nodeOptions nodedriveroptions.NodeOptions, stopCh <-chan struct{}) error { logger := logging.Logger().Sugar() logger.Sync() diff --git a/cmd/oci-csi-node-driver/nodedriveroptions/nodecsioptions.go b/cmd/oci-csi-node-driver/nodedriveroptions/nodecsioptions.go index 8d7b1856db..e6bdbefe02 100644 --- a/cmd/oci-csi-node-driver/nodedriveroptions/nodecsioptions.go +++ b/cmd/oci-csi-node-driver/nodedriveroptions/nodecsioptions.go @@ -14,7 +14,7 @@ package nodedriveroptions -//NodeCSIOptions contains details about the flag +// NodeCSIOptions contains details about the flag type NodeCSIOptions struct { Endpoint string // Used for Block Volume CSI driver NodeID string @@ -22,8 +22,8 @@ type NodeCSIOptions struct { Master string Kubeconfig string - EnableFssDriver bool - FssEndpoint string + EnableFssDriver bool + FssEndpoint string LustreCsiAddress string LustreKubeletRegistrationPath string LustreEndpoint string diff --git a/pkg/cloudprovider/providers/oci/load_balancer_util_test.go b/pkg/cloudprovider/providers/oci/load_balancer_util_test.go index 034abc9b88..1f0bcade0f 100644 --- a/pkg/cloudprovider/providers/oci/load_balancer_util_test.go +++ b/pkg/cloudprovider/providers/oci/load_balancer_util_test.go @@ -25,9 +25,9 @@ import ( api "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/oracle/oci-cloud-controller-manager/pkg/oci/client" "github.com/oracle/oci-go-sdk/v65/common" "github.com/oracle/oci-go-sdk/v65/loadbalancer" - "github.com/oracle/oci-cloud-controller-manager/pkg/oci/client" "github.com/stretchr/testify/assert" ) diff --git a/pkg/csi-util/lustre_lnet_helper.go b/pkg/csi-util/lustre_lnet_helper.go index 4642e6d2cc..311348ee29 100644 --- a/pkg/csi-util/lustre_lnet_helper.go +++ b/pkg/csi-util/lustre_lnet_helper.go @@ -42,7 +42,8 @@ type Parameter map[string]interface{} /* ValidateLustreVolumeId takes lustreVolumeId as input and returns if its valid or not along with lnetLabel Ex. volume handle : 10.112.10.6@tcp1:/fsname - volume handle : [:]:/ + + volume handle : [:]:/ */ func ValidateLustreVolumeId(lusterVolumeId string) (bool, string) { const minNumOfParamsFromVolumeHandle = 2 @@ -91,9 +92,9 @@ type LnetService struct { type OCILnetConfigurator struct{} -func NewLnetService() *LnetService{ +func NewLnetService() *LnetService { return &LnetService{ - Configurator: &OCILnetConfigurator{}, + Configurator: &OCILnetConfigurator{}, } } @@ -331,7 +332,7 @@ func (ls *LnetService) IsLnetActive(logger *zap.SugaredLogger, lnetLabel string) } func (olc *OCILnetConfigurator) ExecuteCommandOnWorkerNode(args ...string) (string, error) { - + command := exec.Command("chroot-bash", args...) output, err := command.CombinedOutput() @@ -374,7 +375,7 @@ func isValidShellInput(input string) bool { return false } // List of forbidden characters - forbiddenChars := []string{";", "&", "|", "<", ">", "(", ")", "`", "'", "\"","$","!"} + forbiddenChars := []string{";", "&", "|", "<", ">", "(", ")", "`", "'", "\"", "$", "!"} for _, char := range forbiddenChars { if strings.Contains(input, char) { return false @@ -382,7 +383,7 @@ func isValidShellInput(input string) bool { } return true } -func ValidateLustreParameters(logger *zap.SugaredLogger, lustreParamsJson string) error { +func ValidateLustreParameters(logger *zap.SugaredLogger, lustreParamsJson string) error { if lustreParamsJson == "" { logger.Debug("No lustre parameters specified.") return nil @@ -401,7 +402,7 @@ func ValidateLustreParameters(logger *zap.SugaredLogger, lustreParamsJson strin for key, value := range param { logger.Infof("Validating lustre param %s=%s", key, fmt.Sprintf("%v", value)) if !isValidShellInput(key) || !isValidShellInput(fmt.Sprintf("%v", value)) { - invalidParams = append(invalidParams, fmt.Sprintf("%v=%v",key, value)) + invalidParams = append(invalidParams, fmt.Sprintf("%v=%v", key, value)) } } } @@ -411,4 +412,3 @@ func ValidateLustreParameters(logger *zap.SugaredLogger, lustreParamsJson strin logger.Infof("Successfully validated lustre parameters.") return nil } - diff --git a/pkg/csi-util/utils.go b/pkg/csi-util/utils.go index 3aed4ad309..51e550d7f8 100644 --- a/pkg/csi-util/utils.go +++ b/pkg/csi-util/utils.go @@ -87,7 +87,6 @@ const ( RawBlockStagingFile = "mountfile" AvailabilityDomainLabel = "csi-ipv6-full-ad-name" - ) // Util interface @@ -117,15 +116,14 @@ type NodeMetadata struct { // CSIConfig represents the structure of the ConfigMap data. type CSIConfig struct { - Lustre *DriverConfig `yaml:"lustre"` + Lustre *DriverConfig `yaml:"lustre"` IsLoaded bool } // DriverConfig represents driver-specific configurations. type DriverConfig struct { - SkipNodeUnstage bool `yaml:"skipNodeUnstage"` + SkipNodeUnstage bool `yaml:"skipNodeUnstage"` SkipLustreParameters bool `yaml:"skipLustreParameters"` - } func (u *Util) LookupNodeID(k kubernetes.Interface, nodeName string) (string, error) { @@ -144,14 +142,14 @@ func (u *Util) LookupNodeID(k kubernetes.Interface, nodeName string) (string, er func (u *Util) WaitForKubeApiServerToBeReachableWithContext(ctx context.Context, k kubernetes.Interface, backOffCap time.Duration) { - waitForKubeApiServerCtx, waitForKubeApiServerCtxCancel := context.WithTimeout(ctx, time.Second * 45) + waitForKubeApiServerCtx, waitForKubeApiServerCtxCancel := context.WithTimeout(ctx, time.Second*45) defer waitForKubeApiServerCtxCancel() backoff := wait.Backoff{ Duration: 1 * time.Second, Factor: 2.0, Steps: 5, - Cap: backOffCap, + Cap: backOffCap, } wait.ExponentialBackoffWithContext( @@ -171,9 +169,9 @@ func (u *Util) WaitForKubeApiServerToBeReachableWithContext(ctx context.Context, ) } -func (u *Util) LoadNodeMetadataFromApiServer(ctx context.Context, k kubernetes.Interface, nodeID string, nodeMetadata *NodeMetadata) (error) { +func (u *Util) LoadNodeMetadataFromApiServer(ctx context.Context, k kubernetes.Interface, nodeID string, nodeMetadata *NodeMetadata) error { - u.WaitForKubeApiServerToBeReachableWithContext(ctx, k, time.Second * 30) + u.WaitForKubeApiServerToBeReachableWithContext(ctx, k, time.Second*30) node, err := k.CoreV1().Nodes().Get(ctx, nodeID, metav1.GetOptions{}) @@ -210,7 +208,7 @@ func (u *Util) LoadNodeMetadataFromApiServer(ctx context.Context, k kubernetes.I u.Logger.With("nodeId", nodeID, "nodeMetadata", nodeMetadata).Info("Node IP family identified.") } nodeMetadata.IsNodeMetadataLoaded = true - return nil + return nil } // waitForPathToExist waits for for a given filesystem path to exist. @@ -652,7 +650,7 @@ func LoadCSIConfigFromConfigMap(csiConfig *CSIConfig, k kubernetes.Interface, co if lustreConfig, exists := cm.Data["lustre"]; exists { if err := yaml.Unmarshal([]byte(lustreConfig), &csiConfig.Lustre); err != nil { - logger.Debugf("Failed to parse lustre key in config map %v. Error: %v",configMapName, err) + logger.Debugf("Failed to parse lustre key in config map %v. Error: %v", configMapName, err) return } logger.Infof("Successfully loaded ConfigMap %v. Using customized configuration for csi driver.", configMapName) diff --git a/pkg/csi-util/utils_test.go b/pkg/csi-util/utils_test.go index dad37fcd59..caefa711b7 100644 --- a/pkg/csi-util/utils_test.go +++ b/pkg/csi-util/utils_test.go @@ -352,21 +352,21 @@ func Test_DiskByPathPatternForPV(t *testing.T) { func Test_LoadNodeMetadataFromApiServer(t *testing.T) { tests := []struct { - name string - nodeName string - want *NodeMetadata - kubeclient kubernetes.Interface - err error + name string + nodeName string + want *NodeMetadata + kubeclient kubernetes.Interface + err error }{ { name: "should return ipv6 for ipv6 preferred node", nodeName: "ipv6Preferred", want: &NodeMetadata{ FullAvailabilityDomain: "xyz:PHX-AD-3", - AvailabilityDomain: "PHX-AD-3", - PreferredNodeIpFamily: Ipv6Stack, - Ipv4Enabled: true, - Ipv6Enabled: true, + AvailabilityDomain: "PHX-AD-3", + PreferredNodeIpFamily: Ipv6Stack, + Ipv4Enabled: true, + Ipv6Enabled: true, }, }, { @@ -374,7 +374,7 @@ func Test_LoadNodeMetadataFromApiServer(t *testing.T) { nodeName: "ipv4Preferred", want: &NodeMetadata{ PreferredNodeIpFamily: Ipv4Stack, - AvailabilityDomain: "PHX-AD-3", + AvailabilityDomain: "PHX-AD-3", Ipv4Enabled: true, Ipv6Enabled: true, }, @@ -383,7 +383,7 @@ func Test_LoadNodeMetadataFromApiServer(t *testing.T) { name: "should return default IPv4 family for no ip preference", nodeName: "noIpPreference", want: &NodeMetadata{ - AvailabilityDomain: "PHX-AD-3", + AvailabilityDomain: "PHX-AD-3", PreferredNodeIpFamily: Ipv4Stack, Ipv4Enabled: true, Ipv6Enabled: false, @@ -410,7 +410,7 @@ func Test_LoadNodeMetadataFromApiServer(t *testing.T) { nodeName: "ipv4Preferred", want: &NodeMetadata{ PreferredNodeIpFamily: Ipv4Stack, - AvailabilityDomain: "PHX-AD-3", + AvailabilityDomain: "PHX-AD-3", Ipv4Enabled: true, Ipv6Enabled: true, }, @@ -424,8 +424,8 @@ func Test_LoadNodeMetadataFromApiServer(t *testing.T) { want: &NodeMetadata{}, err: fmt.Errorf("Failed to get node information from kube api server, please check if kube api server is accessible."), kubeclient: &util.MockKubeClientWithFailingRestClient{ - CoreClient: &util.MockCoreClientWithFailingRestClient{}, - }, + CoreClient: &util.MockCoreClientWithFailingRestClient{}, + }, }, } @@ -438,13 +438,11 @@ func Test_LoadNodeMetadataFromApiServer(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - log.SetOutput(os.Stdout) nodeMetadata := &NodeMetadata{} - ctx, cancel := context.WithTimeout(context.Background(), 10 * time.Second) + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() - var k kubernetes.Interface if tt.kubeclient != nil { k = tt.kubeclient @@ -809,25 +807,24 @@ func Test_ValidateDNSName(t *testing.T) { func Test_LoadCSIConfigFromConfigMap(t *testing.T) { tests := []struct { - name string + name string configMapName string - want *CSIConfig + want *CSIConfig }{ { - name: "Parse Configs correctly when csi config map is present", + name: "Parse Configs correctly when csi config map is present", configMapName: "oci-csi-config", want: &CSIConfig{ Lustre: &DriverConfig{ - SkipNodeUnstage: true, + SkipNodeUnstage: true, SkipLustreParameters: true, }, }, }, { - name: "Return default config if config map is not present", + name: "Return default config if config map is not present", configMapName: "invalid", - want: &CSIConfig{ - }, + want: &CSIConfig{}, }, } diff --git a/pkg/csi/driver/fss_controller.go b/pkg/csi/driver/fss_controller.go index d0f5df6cad..b661ba4c76 100644 --- a/pkg/csi/driver/fss_controller.go +++ b/pkg/csi/driver/fss_controller.go @@ -149,7 +149,7 @@ func (d *FSSControllerDriver) CreateVolume(ctx context.Context, req *csi.CreateV serviceAccountToken = serviceAccountTokenCreated } - ociClientConfig := &client.OCIClientConfig{ SaToken: serviceAccountToken, ParentRptURL: secretParameters.parentRptURL, TenancyId: d.config.Auth.TenancyID } + ociClientConfig := &client.OCIClientConfig{SaToken: serviceAccountToken, ParentRptURL: secretParameters.parentRptURL, TenancyId: d.config.Auth.TenancyID} networkingClient := d.client.Networking(ociClientConfig) if networkingClient == nil { @@ -628,7 +628,7 @@ func extractStorageClassParameters(ctx context.Context, d *FSSControllerDriver, } if client.IsIpv6SingleStackCluster() { - if !strings.Contains(availabilityDomain,":") { + if !strings.Contains(availabilityDomain, ":") { log.Errorf("Full AvailabilityDomain with prefix not provided in storage class for IPv6 single stack cluster.") dimensionsMap[metrics.ComponentDimension] = util.GetMetricDimensionForComponent(util.ErrValidation, util.CSIStorageType) metrics.SendMetricData(d.metricPusher, metrics.FssAllProvision, time.Since(startTime).Seconds(), dimensionsMap) @@ -793,7 +793,7 @@ func provisionMountTarget(ctx context.Context, log *zap.SugaredLogger, c client. SubnetId: &storageClassParameters.mountTargetSubnetOcid, FreeformTags: storageClassParameters.scTags.FreeformTags, DefinedTags: storageClassParameters.scTags.DefinedTags, - NsgIds: storageClassParameters.nsgOcids, + NsgIds: storageClassParameters.nsgOcids, } return fssClient.CreateMountTarget(ctx, createMountTargetDetails) } @@ -833,7 +833,7 @@ func (d *FSSControllerDriver) DeleteVolume(ctx context.Context, req *csi.DeleteV serviceAccountToken = serviceAccountTokenGenerated } - ociClientConfig := &client.OCIClientConfig{ SaToken: serviceAccountToken, ParentRptURL: secretParameters.parentRptURL, TenancyId: d.config.Auth.TenancyID } + ociClientConfig := &client.OCIClientConfig{SaToken: serviceAccountToken, ParentRptURL: secretParameters.parentRptURL, TenancyId: d.config.Auth.TenancyID} fssClient := d.client.FSS(ociClientConfig) @@ -1041,7 +1041,7 @@ func (d *FSSControllerDriver) ValidateVolumeCapabilities(ctx context.Context, re serviceAccountToken = serviceAccountTokenGenerated } - ociClientConfig := &client.OCIClientConfig{ SaToken: serviceAccountToken, ParentRptURL: secretParameters.parentRptURL, TenancyId: d.config.Auth.TenancyID } + ociClientConfig := &client.OCIClientConfig{SaToken: serviceAccountToken, ParentRptURL: secretParameters.parentRptURL, TenancyId: d.config.Auth.TenancyID} networkingClient := d.client.Networking(ociClientConfig) if networkingClient == nil { diff --git a/pkg/csi/driver/fss_controller_test.go b/pkg/csi/driver/fss_controller_test.go index 4a2dbb4cad..be41a506bd 100644 --- a/pkg/csi/driver/fss_controller_test.go +++ b/pkg/csi/driver/fss_controller_test.go @@ -543,12 +543,12 @@ func TestFSSControllerDriver_CreateVolume(t *testing.T) { wantErr: errors.New("Neither Mount Target Ocid nor Mount Target Subnet Ocid provided in storage class"), }, { - name: "Error when invalid JSON string provided for mount target NSGs", + name: "Error when invalid JSON string provided for mount target NSGs", fields: fields{}, args: args{ ctx: context.Background(), req: &csi.CreateVolumeRequest{ - Name: "ut-volume", + Name: "ut-volume", Parameters: map[string]string{"availabilityDomain": "US-ASHBURN-AD-1", "mountTargetSubnetOcid": "oc1.subnet.xxxx", "nsgOcids": ""}, VolumeCapabilities: []*csi.VolumeCapability{{ AccessMode: &csi.VolumeCapability_AccessMode{ @@ -557,7 +557,7 @@ func TestFSSControllerDriver_CreateVolume(t *testing.T) { }}, }, }, - want: nil, + want: nil, wantErr: errors.New("Failed to parse nsgOcids provided in storage class. Please provide valid input."), }, { diff --git a/pkg/csi/driver/lustre_node.go b/pkg/csi/driver/lustre_node.go index 391fa645d4..34af114dd1 100644 --- a/pkg/csi/driver/lustre_node.go +++ b/pkg/csi/driver/lustre_node.go @@ -41,7 +41,6 @@ func (d LustreNodeDriver) NodeStageVolume(ctx context.Context, req *csi.NodeStag return nil, status.Error(codes.InvalidArgument, "Invalid Volume Handle provided.") } - d.loadCSIConfig() if lustrePostMountParameters, exists := req.GetVolumeContext()["lustrePostMountParameters"]; exists && !isSkipLustreParams(d.csiConfig) { @@ -65,7 +64,7 @@ func (d LustreNodeDriver) NodeStageVolume(ctx context.Context, req *csi.NodeStag //Lnet Setup if setupLnet, ok := req.GetVolumeContext()[SetupLnet]; ok && setupLnet == "true" { - lustreSubnetCIDR, ok := req.GetVolumeContext()[LustreSubnetCidr] + lustreSubnetCIDR, ok := req.GetVolumeContext()[LustreSubnetCidr] if !ok { lustreSubnetCIDR = fmt.Sprintf("%s/32", d.nodeID) @@ -90,8 +89,6 @@ func (d LustreNodeDriver) NodeStageVolume(ctx context.Context, req *csi.NodeStag mounter := mount.New(mountPath) - - targetPath := req.StagingTargetPath mountPoint, err := isMountPoint(mounter, targetPath) if err != nil { @@ -130,7 +127,7 @@ func (d LustreNodeDriver) NodeStageVolume(ctx context.Context, req *csi.NodeStag if lustrePostMountParameters, exists := req.GetVolumeContext()["lustrePostMountParameters"]; exists { d.loadCSIConfig() - if !isSkipLustreParams(d.csiConfig) { + if !isSkipLustreParams(d.csiConfig) { err = lnetService.ApplyLustreParameters(logger, lustrePostMountParameters) if err != nil { //Unmounting volume on error as we are failing NodeStageVolume. If we don't unmount and customer deletes workload then volume will remain mounted as NodeUnstageVolume won't be called. @@ -156,7 +153,6 @@ func (d LustreNodeDriver) loadCSIConfig() { d.csiConfig.IsLoaded = true } - func (d LustreNodeDriver) NodeUnstageVolume(ctx context.Context, req *csi.NodeUnstageVolumeRequest) (*csi.NodeUnstageVolumeResponse, error) { if req.VolumeId == "" { @@ -214,7 +210,7 @@ func (d LustreNodeDriver) NodeUnstageVolume(ctx context.Context, req *csi.NodeUn logger.With("StagingTargetPath", targetPath).Infof("mount point does not exist") return &csi.NodeUnstageVolumeResponse{}, nil } - return nil, status.Error(codes.Internal, err.Error()) + return nil, status.Error(codes.Internal, err.Error()) } if !isMountPoint { @@ -222,7 +218,7 @@ func (d LustreNodeDriver) NodeUnstageVolume(ctx context.Context, req *csi.NodeUn err = os.RemoveAll(targetPath) if err != nil { logger.With(zap.Error(err)).Error("Remove target path failed with error") - return nil, status.Error(codes.Internal, "Failed to remove target path") + return nil, status.Error(codes.Internal, "Failed to remove target path") } return &csi.NodeUnstageVolumeResponse{}, nil } @@ -250,7 +246,6 @@ func (d LustreNodeDriver) NodePublishVolume(ctx context.Context, req *csi.NodePu return nil, status.Error(codes.InvalidArgument, "Target Path must be provided") } - logger := d.logger.With("volumeID", req.VolumeId) logger.Debugf("volume context: %v", req.VolumeContext) diff --git a/pkg/oci/client/block_storage.go b/pkg/oci/client/block_storage.go index 0d612b57cf..7924e052c8 100644 --- a/pkg/oci/client/block_storage.go +++ b/pkg/oci/client/block_storage.go @@ -99,7 +99,7 @@ func (c *client) GetBootVolume(ctx context.Context, id string) (*core.BootVolume } resp, err := c.bs.GetBootVolume(ctx, core.GetBootVolumeRequest{ - BootVolumeId: &id, + BootVolumeId: &id, RequestMetadata: c.requestMetadata}) incRequestCounter(err, getVerb, volumeResource) diff --git a/pkg/oci/client/client_test.go b/pkg/oci/client/client_test.go index 7960981ec7..b11454f848 100644 --- a/pkg/oci/client/client_test.go +++ b/pkg/oci/client/client_test.go @@ -194,8 +194,8 @@ func (c *mockComputeClient) ListInstanceDevices(ctx context.Context, request cor } else if *request.InstanceId == "ocid1.one-device-path-available" { return core.ListInstanceDevicesResponse{ Items: []core.Device{{ - Name: &devicePath, - }, + Name: &devicePath, + }, }, }, nil } diff --git a/pkg/oci/client/generic_load_balancer_types.go b/pkg/oci/client/generic_load_balancer_types.go index 5cd5adb590..d13ac156e7 100644 --- a/pkg/oci/client/generic_load_balancer_types.go +++ b/pkg/oci/client/generic_load_balancer_types.go @@ -118,7 +118,7 @@ type GenericCreateLoadBalancerDetails struct { // Only needed for LB Certificates map[string]GenericCertificate - RuleSets map[string]loadbalancer.RuleSetDetails + RuleSets map[string]loadbalancer.RuleSetDetails // Supported only in NLB AssignedPrivateIpv4 *string AssignedIpv6 *string diff --git a/pkg/oci/client/volume_attachment_test.go b/pkg/oci/client/volume_attachment_test.go index 2c7739b739..410bde30ff 100644 --- a/pkg/oci/client/volume_attachment_test.go +++ b/pkg/oci/client/volume_attachment_test.go @@ -11,28 +11,27 @@ import ( func Test_getDevicePath(t *testing.T) { var tests = map[string]struct { - instanceID string - want string - wantErr error + instanceID string + want string + wantErr error }{ "getDevicePathNoDeviceAvailable": { instanceID: "ocid1.device-path-not-available", - wantErr: fmt.Errorf("Max number of volumes are already attached to instance %s. Please schedule workload on different node.", "ocid1.device-path-not-available"), - + wantErr: fmt.Errorf("Max number of volumes are already attached to instance %s. Please schedule workload on different node.", "ocid1.device-path-not-available"), }, "getDevicePathOneDeviceAvailable": { instanceID: "ocid1.one-device-path-available", - want: "/dev/oracleoci/oraclevdac", + want: "/dev/oracleoci/oraclevdac", }, "getDevicePathReturnsError": { instanceID: "ocid1.device-path-returns-error", - wantErr: errNotFound, + wantErr: errNotFound, }, } - + vaClient := &client{ compute: &mockComputeClient{}, - logger: zap.S(), + logger: zap.S(), } for name, tc := range tests { diff --git a/pkg/util/disk/iscsi.go b/pkg/util/disk/iscsi.go index a60cf858e1..fda916ca4d 100644 --- a/pkg/util/disk/iscsi.go +++ b/pkg/util/disk/iscsi.go @@ -90,7 +90,7 @@ type Interface interface { DeviceOpened(pathname string) (bool, error) - IsMounted(devicePath string, targetPath string) (bool, error) + IsMounted(devicePath string, targetPath string) (bool, error) // updates the queue depth for iSCSI target UpdateQueueDepth() error @@ -494,7 +494,7 @@ func (c *iSCSIMounter) IsMounted(devicePath string, targetPath string) (bool, er var diskByPath string notMnt, err := c.mounter.IsLikelyNotMountPoint(targetPath) if err != nil { - if os.IsNotExist(err){ + if os.IsNotExist(err) { return false, nil } return false, fmt.Errorf("failed to check if %s is a mount point: %v", targetPath, err) diff --git a/pkg/util/disk/paravirtualized.go b/pkg/util/disk/paravirtualized.go index c2f5288fde..87ff5c34b4 100644 --- a/pkg/util/disk/paravirtualized.go +++ b/pkg/util/disk/paravirtualized.go @@ -104,7 +104,7 @@ func (c *pvMounter) DeviceOpened(pathname string) (bool, error) { func (c *pvMounter) IsMounted(devicePath string, targetPath string) (bool, error) { notMnt, err := c.mounter.IsLikelyNotMountPoint(targetPath) if err != nil { - if os.IsNotExist(err){ + if os.IsNotExist(err) { return false, nil } return false, fmt.Errorf("failed to check if %s is a mount point: %v", targetPath, err) diff --git a/pkg/util/osinfo/osinfo.go b/pkg/util/osinfo/osinfo.go index b7753051f7..7e84de9f5c 100644 --- a/pkg/util/osinfo/osinfo.go +++ b/pkg/util/osinfo/osinfo.go @@ -12,9 +12,9 @@ var OsName = "" const ( LinuxOsReleaseFile = "/host/etc/os-release" - DebianOSName = "Debian GNU/Linux" + DebianOSName = "Debian GNU/Linux" - UbuntuOSName = "Ubuntu" + UbuntuOSName = "Ubuntu" ) func GetOsName() (name string) { @@ -22,8 +22,8 @@ func GetOsName() (name string) { return OsName } - OsName = parseLinuxReleaseFile(LinuxOsReleaseFile) - return OsName; + OsName = parseLinuxReleaseFile(LinuxOsReleaseFile) + return OsName } func readLines(path string) ([]string, error) { @@ -58,7 +58,6 @@ func parseLinuxReleaseFile(releaseFile string) (name string) { return osName } - func IsUbuntu() bool { return strings.EqualFold(UbuntuOSName, GetOsName()) } diff --git a/pkg/util/signals/signal_posix.go b/pkg/util/signals/signal_posix.go index 9bdb4e7418..a0f00a7321 100644 --- a/pkg/util/signals/signal_posix.go +++ b/pkg/util/signals/signal_posix.go @@ -1,3 +1,4 @@ +//go:build !windows // +build !windows /* diff --git a/test/e2e/cloud-provider-oci/lustre_static.go b/test/e2e/cloud-provider-oci/lustre_static.go index 8bd7a184c0..38b30f9368 100644 --- a/test/e2e/cloud-provider-oci/lustre_static.go +++ b/test/e2e/cloud-provider-oci/lustre_static.go @@ -26,9 +26,9 @@ var _ = Describe("Lustre Static", func() { It("Multiple Pods should be able consume same PVC and read, write to same file", func() { pvcJig := framework.NewPVCTestJig(f.ClientSet, "csi-lustre-e2e-test") - pvVolumeAttributes := map[string]string{ "setupLnet": "true"} + pvVolumeAttributes := map[string]string{"setupLnet": "true"} if setupF.LustreSubnetCidr != "" { - pvVolumeAttributes["lustreSubnetCidr"]= setupF.LustreSubnetCidr + pvVolumeAttributes["lustreSubnetCidr"] = setupF.LustreSubnetCidr } pv := pvcJig.CreatePVorFailLustre(f.Namespace.Name, setupF.LustreVolumeHandle, []string{}, pvVolumeAttributes) @@ -55,9 +55,9 @@ var _ = Describe("Lustre Static", func() { //LUSTRE lusterPVCJig := framework.NewPVCTestJig(f.ClientSet, "csi-lustre-e2e-test") - pvVolumeAttributes := map[string]string{ "setupLnet": "true"} + pvVolumeAttributes := map[string]string{"setupLnet": "true"} if setupF.LustreSubnetCidr != "" { - pvVolumeAttributes["lustreSubnetCidr"]= setupF.LustreSubnetCidr + pvVolumeAttributes["lustreSubnetCidr"] = setupF.LustreSubnetCidr } lustrePV := lusterPVCJig.CreatePVorFailLustre(f.Namespace.Name, setupF.LustreVolumeHandle, []string{}, pvVolumeAttributes) lustrePVC := lusterPVCJig.CreateAndAwaitPVCOrFailStaticLustre(f.Namespace.Name, lustrePV.Name, "50Gi", nil) @@ -72,9 +72,9 @@ var _ = Describe("Lustre Static", func() { It("Create PV PVC and POD for CSI-Lustre with mount options", func() { pvcJig := framework.NewPVCTestJig(f.ClientSet, "csi-lustre-e2e-test") mountOptions := []string{"flock"} - pvVolumeAttributes := map[string]string{ "setupLnet": "true"} + pvVolumeAttributes := map[string]string{"setupLnet": "true"} if setupF.LustreSubnetCidr != "" { - pvVolumeAttributes["lustreSubnetCidr"]= setupF.LustreSubnetCidr + pvVolumeAttributes["lustreSubnetCidr"] = setupF.LustreSubnetCidr } pv := pvcJig.CreatePVorFailLustre(f.Namespace.Name, setupF.LustreVolumeHandle, mountOptions, pvVolumeAttributes) @@ -85,7 +85,7 @@ var _ = Describe("Lustre Static", func() { It("Create PV PVC and POD for CSI-Lustre with lustre post mount parameters", func() { pvcJig := framework.NewPVCTestJig(f.ClientSet, "csi-lustre-e2e-test") - pvVolumeAttributes := map[string]string{ "setupLnet": "true", "lustrePostMountParameters" : "[{\"*.*.*MDT*.lru_size\" : 11201}]"} + pvVolumeAttributes := map[string]string{"setupLnet": "true", "lustrePostMountParameters": "[{\"*.*.*MDT*.lru_size\" : 11201}]"} pv := pvcJig.CreatePVorFailLustre(f.Namespace.Name, setupF.LustreVolumeHandle, []string{}, pvVolumeAttributes) pvc := pvcJig.CreateAndAwaitPVCOrFailStaticLustre(f.Namespace.Name, pv.Name, "50Gi", nil) @@ -98,9 +98,9 @@ var _ = Describe("Lustre Static", func() { It("Verify volume group ownership change for Lustre when fsGroup is defined", func() { pvcJig := framework.NewPVCTestJig(f.ClientSet, "csi-lustre-e2e-test") - pvVolumeAttributes := map[string]string{ "setupLnet": "true"} + pvVolumeAttributes := map[string]string{"setupLnet": "true"} if setupF.LustreSubnetCidr != "" { - pvVolumeAttributes["lustreSubnetCidr"]= setupF.LustreSubnetCidr + pvVolumeAttributes["lustreSubnetCidr"] = setupF.LustreSubnetCidr } pv := pvcJig.CreatePVorFailLustre(f.Namespace.Name, setupF.LustreVolumeHandle, []string{}, pvVolumeAttributes) diff --git a/test/e2e/framework/framework.go b/test/e2e/framework/framework.go index 1b979bf426..81cb029234 100644 --- a/test/e2e/framework/framework.go +++ b/test/e2e/framework/framework.go @@ -43,22 +43,22 @@ const ( DefaultClusterKubeconfig = "/tmp/clusterkubeconfig" DefaultCloudConfig = "/tmp/cloudconfig" - ClassOCI = "oci" - ClassOCICSI = "oci-bv" - ClassCustom = "oci-bv-custom" - ClassOCICSIExpand = "oci-bv-expand" - ClassOCILowCost = "oci-bv-low" - ClassOCIBalanced = "oci-bal" - ClassOCIHigh = "oci-bv-high" - ClassOCIUHP = "oci-uhp" - ClassOCIKMS = "oci-kms" - ClassOCIExt3 = "oci-ext3" - ClassOCIXfs = "oci-xfs" - ClassFssDynamic = "oci-file-storage-test" - ClassSnapshot = "oci-snapshot-sc" - MinVolumeBlock = "50Gi" - MaxVolumeBlock = "100Gi" - VolumeFss = "1Gi" + ClassOCI = "oci" + ClassOCICSI = "oci-bv" + ClassCustom = "oci-bv-custom" + ClassOCICSIExpand = "oci-bv-expand" + ClassOCILowCost = "oci-bv-low" + ClassOCIBalanced = "oci-bal" + ClassOCIHigh = "oci-bv-high" + ClassOCIUHP = "oci-uhp" + ClassOCIKMS = "oci-kms" + ClassOCIExt3 = "oci-ext3" + ClassOCIXfs = "oci-xfs" + ClassFssDynamic = "oci-file-storage-test" + ClassSnapshot = "oci-snapshot-sc" + MinVolumeBlock = "50Gi" + MaxVolumeBlock = "100Gi" + VolumeFss = "1Gi" VSClassDefault = "oci-snapclass" NodeHostnameLabel = "kubernetes.io/hostname" @@ -87,12 +87,12 @@ var ( reservedIP string // Testing public reserved IP feature architecture string volumeHandle string // The FSS mount volume handle - lustreVolumeHandle string // The Lustre mount volume handle + lustreVolumeHandle string // The Lustre mount volume handle lustreSubnetCidr string // The Lustre Subnet Cidr staticSnapshotCompartmentOCID string // Compartment ID for cross compartment snapshot test - customDriverHandle string // Custom driver handle for custom CSI driver installation + customDriverHandle string // Custom driver handle for custom CSI driver installation runUhpE2E bool // Whether to run UHP E2Es, requires Volume Management Plugin enabled on the node and 16+ cores (check blockvolumeperformance public doc for the exact requirements) - enableParallelRun bool + enableParallelRun bool addOkeSystemTags bool clusterID string // Ocid of the newly created E2E cluster clusterType string // Cluster type can be BASIC_CLUSTER or ENHANCED_CLUSTER (Default: BASIC_CLUSTER) @@ -166,7 +166,7 @@ type Framework struct { ReservedIP string Architecture string - VolumeHandle string + VolumeHandle string LustreVolumeHandle string LustreSubnetCidr string @@ -174,10 +174,10 @@ type Framework struct { // Compartment ID for cross compartment snapshot test StaticSnapshotCompartmentOcid string RunUhpE2E bool - CustomDriverHandle string - BlockProvisionerName string - FSSProvisionerName string - AddOkeSystemTags bool + CustomDriverHandle string + BlockProvisionerName string + FSSProvisionerName string + AddOkeSystemTags bool } // New creates a new a framework that holds the context of the test @@ -204,7 +204,7 @@ func NewWithConfig() *Framework { LustreSubnetCidr: lustreSubnetCidr, StaticSnapshotCompartmentOcid: staticSnapshotCompartmentOCID, RunUhpE2E: runUhpE2E, - CustomDriverHandle: customDriverHandle, + CustomDriverHandle: customDriverHandle, AddOkeSystemTags: addOkeSystemTags, ClusterType: clusterTypeEnum, } diff --git a/test/e2e/framework/volumesnapshotclass_util.go b/test/e2e/framework/volumesnapshotclass_util.go index 3b3d11916a..b5c5cc4afa 100644 --- a/test/e2e/framework/volumesnapshotclass_util.go +++ b/test/e2e/framework/volumesnapshotclass_util.go @@ -49,17 +49,17 @@ func (f *CloudProviderFramework) CreateVolumeSnapshotClassOrFail(name string, dr // does not actually create the storage class. The default storage class has the same name // as the jig func (f *CloudProviderFramework) NewVolumeSnapshotClassTemplate(name string, parameters map[string]string, - driverType string,deletionPolicy snapshot.DeletionPolicy) *snapshot.VolumeSnapshotClass { + driverType string, deletionPolicy snapshot.DeletionPolicy) *snapshot.VolumeSnapshotClass { return &snapshot.VolumeSnapshotClass{ TypeMeta: metav1.TypeMeta{ Kind: "VolumeSnapshotClass", APIVersion: "snapshot.storage.k8s.io/v1", }, ObjectMeta: metav1.ObjectMeta{ - Name: name, + Name: name, }, Driver: driverType, - Parameters: parameters, + Parameters: parameters, DeletionPolicy: deletionPolicy, } } From 4e6003ac7a7b27df6eeb97cd4c2f3c000526883f Mon Sep 17 00:00:00 2001 From: Hadrien Patte Date: Mon, 5 Jan 2026 15:39:53 +0100 Subject: [PATCH 2/5] External-ccm: Fix `go vet` issues Signed-off-by: Hadrien Patte --- pkg/cloudprovider/providers/oci/instances_test.go | 6 +++--- pkg/csi/driver/fss_controller.go | 4 ++-- pkg/oci/client/load_balancer_test.go | 5 +++-- pkg/oci/client/network_load_balancer_test.go | 5 +++-- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/pkg/cloudprovider/providers/oci/instances_test.go b/pkg/cloudprovider/providers/oci/instances_test.go index d86ab909f1..be6fb93a00 100644 --- a/pkg/cloudprovider/providers/oci/instances_test.go +++ b/pkg/cloudprovider/providers/oci/instances_test.go @@ -538,7 +538,7 @@ var ( Status: v1.PodStatus{ PodIP: "0.0.0.10", PodIPs: []v1.PodIP{ - {"0.0.0.10"}, + {IP: "0.0.0.10"}, }, }, }, @@ -555,8 +555,8 @@ var ( Status: v1.PodStatus{ PodIP: "0.0.0.20", PodIPs: []v1.PodIP{ - {"0.0.0.20"}, - {"2001:0db8:85a3:0000:0000:8a2e:0370:7334"}, + {IP: "0.0.0.20"}, + {IP: "2001:0db8:85a3:0000:0000:8a2e:0370:7334"}, }, }, }, diff --git a/pkg/csi/driver/fss_controller.go b/pkg/csi/driver/fss_controller.go index b661ba4c76..8ae633975f 100644 --- a/pkg/csi/driver/fss_controller.go +++ b/pkg/csi/driver/fss_controller.go @@ -252,8 +252,8 @@ func checkForSupportedVolumeCapabilities(volumeCaps []*csi.VolumeCapability) err if blk := cap.GetBlock(); blk != nil { return fmt.Errorf("driver does not support block volumes") } - for _, c := range fssSupportedVolumeCapabilities { - if c.GetMode() == cap.AccessMode.GetMode() { + for i := range fssSupportedVolumeCapabilities { + if fssSupportedVolumeCapabilities[i].GetMode() == cap.AccessMode.GetMode() { return nil } } diff --git a/pkg/oci/client/load_balancer_test.go b/pkg/oci/client/load_balancer_test.go index e82103dd4a..cbe6e61294 100644 --- a/pkg/oci/client/load_balancer_test.go +++ b/pkg/oci/client/load_balancer_test.go @@ -32,11 +32,12 @@ import ( var TestNonRetryableError = errors.New("some non-retryable error") func TestLB_AwaitWorkRequest(t *testing.T) { - var tests = map[string]struct { + type testCase struct { skip bool // set true to skip a test-case loadbalancer loadbalancerClientStruct wantErr error - }{ + } + var tests = map[string]*testCase{ "getWorkRequestTimedOut": { skip: true, loadbalancer: loadbalancerClientStruct{ diff --git a/pkg/oci/client/network_load_balancer_test.go b/pkg/oci/client/network_load_balancer_test.go index d98ce9645e..d2f0f16a99 100644 --- a/pkg/oci/client/network_load_balancer_test.go +++ b/pkg/oci/client/network_load_balancer_test.go @@ -31,11 +31,12 @@ import ( ) func TestNLB_AwaitWorkRequest(t *testing.T) { - var tests = map[string]struct { + type testCase struct { skip bool // set true to skip a test-case loadbalancer networkLoadbalancer wantErr error - }{ + } + var tests = map[string]*testCase{ "getWorkRequestTimedOut": { skip: true, loadbalancer: networkLoadbalancer{ From 546ff0a2db69fbae7e80bf6116aa24d6422cf4b8 Mon Sep 17 00:00:00 2001 From: bob Date: Wed, 18 Dec 2024 17:55:52 +0100 Subject: [PATCH 3/5] Make the workload identity authentication work. This is WIP --- .../providers/oci/config/config.go | 20 ++++++++++++++++ .../providers/oci/config/config_validate.go | 8 ++++++- .../oci/config/config_validate_test.go | 24 ++++++++++++++++--- 3 files changed, 48 insertions(+), 4 deletions(-) diff --git a/pkg/cloudprovider/providers/oci/config/config.go b/pkg/cloudprovider/providers/oci/config/config.go index ddc6b439b9..4d7fced5e8 100644 --- a/pkg/cloudprovider/providers/oci/config/config.go +++ b/pkg/cloudprovider/providers/oci/config/config.go @@ -15,6 +15,7 @@ package config import ( + "fmt" "github.com/oracle/oci-cloud-controller-manager/pkg/oci/instance/metadata" "io" "os" @@ -144,6 +145,10 @@ type Config struct { // When set to true, clients will use an instance principal configuration provider and ignore auth fields. UseInstancePrincipals bool `yaml:"useInstancePrincipals"` + + // When set to true, clients will use OKE workload identity and ignore auth fields. + UseWorkloadIdentity bool `yaml:"useWorkloadIdentity"` + // CompartmentID is the OCID of the Compartment within which the cluster // resides. CompartmentID string `yaml:"compartment"` @@ -278,6 +283,21 @@ func NewConfigurationProvider(cfg *Config) (common.ConfigurationProvider, error) return cp, nil } + if cfg.UseWorkloadIdentity { + // OCI SDK requires specific, dynamic environment variables for workload identity. + if err := os.Setenv(auth.ResourcePrincipalVersionEnvVar, auth.ResourcePrincipalVersion2_2); err != nil { + return nil, fmt.Errorf("unable to set OCI SDK environment variable: %s: %w", auth.ResourcePrincipalVersionEnvVar, err) + } + if err := os.Setenv(auth.ResourcePrincipalRegionEnvVar, cfg.RegionKey); err != nil { + return nil, fmt.Errorf("unable to set OCI SDK environment variable: %s: %w", auth.ResourcePrincipalRegionEnvVar, err) + } + cp, err := auth.OkeWorkloadIdentityConfigurationProvider() + if err != nil { + return nil, fmt.Errorf("unable to load workload-identity auth method. %v", err) + } + return cp, nil + } + conf = common.NewRawConfigurationProvider( cfg.Auth.TenancyID, cfg.Auth.UserID, diff --git a/pkg/cloudprovider/providers/oci/config/config_validate.go b/pkg/cloudprovider/providers/oci/config/config_validate.go index a5b4879538..8e59fcb338 100644 --- a/pkg/cloudprovider/providers/oci/config/config_validate.go +++ b/pkg/cloudprovider/providers/oci/config/config_validate.go @@ -76,9 +76,15 @@ func ValidateConfig(c *Config) field.ErrorList { if len(c.CompartmentID) == 0 { allErrs = append(allErrs, field.InternalError(field.NewPath("compartment"), errors.New("This value is normally discovered automatically if omitted. Continue checking the logs to see if something else is wrong"))) } - if !c.UseInstancePrincipals { + + if c.UseWorkloadIdentity && c.UseInstancePrincipals { + allErrs = append(allErrs, field.Forbidden(field.NewPath("useWorkloadIdentity"), "useWorkloadIdentity and useInstancePrincipals cannot be used together")) + } + + if !c.UseInstancePrincipals && !c.UseWorkloadIdentity { allErrs = append(allErrs, validateAuthConfig(&c.Auth, field.NewPath("auth"))...) } + if c.LoadBalancer != nil && !c.LoadBalancer.Disabled { allErrs = append(allErrs, validateLoadBalancerConfig(c, field.NewPath("loadBalancer"))...) } diff --git a/pkg/cloudprovider/providers/oci/config/config_validate_test.go b/pkg/cloudprovider/providers/oci/config/config_validate_test.go index e44856c776..0be11ade4b 100644 --- a/pkg/cloudprovider/providers/oci/config/config_validate_test.go +++ b/pkg/cloudprovider/providers/oci/config/config_validate_test.go @@ -54,16 +54,34 @@ func TestValidateConfig(t *testing.T) { in: &Config{ metadataSvc: metadata.NewMock(&metadata.InstanceMetadata{CompartmentID: "compartment"}), Auth: AuthConfig{ - metadataSvc: metadata.NewMock(&metadata.InstanceMetadata{CompartmentID: "compartment"}), - UseInstancePrincipals: true, - TenancyID: "not empty", + metadataSvc: metadata.NewMock(&metadata.InstanceMetadata{CompartmentID: "compartment"}), + TenancyID: "not empty", }, + UseInstancePrincipals: true, LoadBalancer: &LoadBalancerConfig{ Subnet1: "ocid1.tenancy.oc1..aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", Subnet2: "ocid1.subnet.oc1.phx.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", }, }, errs: field.ErrorList{}, + }, { + name: "invalid with instance principals & workload identity", + in: &Config{ + metadataSvc: metadata.NewMock(&metadata.InstanceMetadata{CompartmentID: "compartment"}), + UseWorkloadIdentity: true, + UseInstancePrincipals: true, + Auth: AuthConfig{ + metadataSvc: metadata.NewMock(&metadata.InstanceMetadata{CompartmentID: "compartment"}), + TenancyID: "not empty", + }, + LoadBalancer: &LoadBalancerConfig{ + Subnet1: "ocid1.tenancy.oc1..aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + Subnet2: "ocid1.subnet.oc1.phx.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + }, + }, + errs: field.ErrorList{ + &field.Error{Type: field.ErrorTypeForbidden, BadValue: "", Field: "useWorkloadIdentity", Detail: "useWorkloadIdentity and useInstancePrincipals cannot be used together"}, + }, }, { name: "valid_with_non_default_security_list_management_mode", in: &Config{ From ba629689e958d140b8ac22f0dd34537ed8f65a01 Mon Sep 17 00:00:00 2001 From: Hadrien Patte Date: Tue, 13 Jan 2026 15:26:53 +0100 Subject: [PATCH 4/5] External-ccm: Implement IPAM Signed-off-by: Hadrien Patte --- pkg/cloudprovider/providers/oci/ccm.go | 30 + .../providers/oci/config/config.go | 47 ++ .../providers/oci/instances_test.go | 8 +- .../providers/oci/ipam/cidr_allocator.go | 237 +++++++ .../providers/oci/ipam/cloud_allocator.go | 633 ++++++++++++++++++ pkg/cloudprovider/providers/oci/ipam/types.go | 53 ++ pkg/cloudprovider/providers/oci/routes.go | 259 +++++++ pkg/csi/driver/bv_controller_test.go | 8 +- pkg/oci/client/client.go | 2 + pkg/oci/client/client_test.go | 4 + pkg/oci/client/compute.go | 64 ++ pkg/volume/provisioner/block/block_test.go | 8 +- pkg/volume/provisioner/fss/fss_test.go | 8 +- 13 files changed, 1353 insertions(+), 8 deletions(-) create mode 100644 pkg/cloudprovider/providers/oci/ipam/cidr_allocator.go create mode 100644 pkg/cloudprovider/providers/oci/ipam/cloud_allocator.go create mode 100644 pkg/cloudprovider/providers/oci/ipam/types.go create mode 100644 pkg/cloudprovider/providers/oci/routes.go diff --git a/pkg/cloudprovider/providers/oci/ccm.go b/pkg/cloudprovider/providers/oci/ccm.go index d6a620e9f6..ae73aacaf4 100644 --- a/pkg/cloudprovider/providers/oci/ccm.go +++ b/pkg/cloudprovider/providers/oci/ccm.go @@ -33,6 +33,7 @@ import ( cloudprovider "k8s.io/cloud-provider" providercfg "github.com/oracle/oci-cloud-controller-manager/pkg/cloudprovider/providers/oci/config" + "github.com/oracle/oci-cloud-controller-manager/pkg/cloudprovider/providers/oci/ipam" "github.com/oracle/oci-cloud-controller-manager/pkg/metrics" "github.com/oracle/oci-cloud-controller-manager/pkg/oci/client" "github.com/oracle/oci-cloud-controller-manager/pkg/oci/instance/metadata" @@ -192,6 +193,31 @@ func (cp *CloudProvider) Initialize(clientBuilder cloudprovider.ControllerClient go nodeInfoController.Run(wait.NeverStop) + // Initialize IPAM controller if enabled + if cp.config.IPAM != nil && cp.config.IPAM.EnableIPAM { + cp.logger.Info("Initializing OCI IPAM controller") + + ipamController := ipam.NewCloudAllocator( + cp.kubeclient, + cp.client, + nodeInformer, + cp.config.IPAM.NodeCIDRMaskSizeIPv4, + cp.config.IPAM.PodSubnetIDs, + cp.config.IPAM.AutoAttachPodVNIC, + cp.config.IPAM.PodVNICDisplayName, + cp.logger, + ) + + // Start IPAM controller with 2 workers + go func() { + if err := ipamController.Run(context.Background(), 2); err != nil { + cp.logger.With("error", err).Error("IPAM controller exited with error") + } + }() + + cp.logger.Info("OCI IPAM controller started") + } + cp.logger.Info("Waiting for node informer cache to sync") if !cache.WaitForCacheSync(wait.NeverStop, nodeInformer.Informer().HasSynced, serviceInformer.Informer().HasSynced) { utilruntime.HandleError(fmt.Errorf("Timed out waiting for informers to sync")) @@ -263,6 +289,10 @@ func (cp *CloudProvider) Clusters() (cloudprovider.Clusters, bool) { // Routes returns a routes interface along with whether the interface is // supported. func (cp *CloudProvider) Routes() (cloudprovider.Routes, bool) { + if cp.config.IPAM != nil && cp.config.IPAM.EnableIPAM { + cp.logger.Debug("Routes interface is supported (IPAM enabled)") + return &routes{cp: cp}, true + } return nil, false } diff --git a/pkg/cloudprovider/providers/oci/config/config.go b/pkg/cloudprovider/providers/oci/config/config.go index 4d7fced5e8..f70f540d58 100644 --- a/pkg/cloudprovider/providers/oci/config/config.go +++ b/pkg/cloudprovider/providers/oci/config/config.go @@ -130,6 +130,33 @@ type InitialTags struct { Common *TagConfig `yaml:"common"` } +// IPAMConfig holds the configuration options for IPAM (IP Address Management) +// support in the OCI CCM. +type IPAMConfig struct { + // EnableIPAM enables IPAM support in the CCM. When enabled, the CCM will + // allocate pod CIDRs to nodes and manage routing via secondary VNICs. + EnableIPAM bool `yaml:"enableIPAM"` + + // PodSubnetIDs maps availability domain names to pod subnet OCIDs. + // Each node in an AD will have a secondary VNIC attached in the corresponding pod subnet. + // Example: {"zkJl:US-ASHBURN-AD-1": "ocid1.subnet.oc1.iad.aaa..."} + PodSubnetIDs map[string]string `yaml:"podSubnetIds"` + + // NodeCIDRMaskSizeIPv4 is the mask size for IPv4 node CIDRs (default: 24). + // This determines how large each node's pod CIDR will be. + // For example, 24 means each node gets a /24 (256 IPs) from the pod subnet. + NodeCIDRMaskSizeIPv4 int `yaml:"nodeCIDRMaskSizeIPv4"` + + // AutoAttachPodVNIC if true, CCM will automatically attach secondary VNICs + // to nodes in pod subnets. If false, assumes VNICs are pre-attached. + AutoAttachPodVNIC bool `yaml:"autoAttachPodVNIC"` + + // PodVNICDisplayName is the display name pattern for pod VNICs (default: "pod-vnic"). + // Used to identify pod VNICs when AutoAttachPodVNIC is enabled or when validating + // existing VNIC attachments. + PodVNICDisplayName string `yaml:"podVNICDisplayName"` +} + // Config holds the OCI cloud-provider config passed to Kubernetes components // via the --cloud-config option. type Config struct { @@ -140,6 +167,8 @@ type Config struct { Metrics *MetricsConfig `yaml:"metrics"` // Tags to be added to managed LB and BV Tags *InitialTags `yaml:"tags"` + // IPAM configuration for pod CIDR allocation and management + IPAM *IPAMConfig `yaml:"ipam"` RegionKey string `yaml:"regionKey"` @@ -175,6 +204,21 @@ func (c *LoadBalancerConfig) Complete() { } } +// Complete the IPAM config applying defaults / overrides. +func (c *IPAMConfig) Complete() { + if !c.EnableIPAM { + return + } + // Set default node CIDR mask size if not specified + if c.NodeCIDRMaskSizeIPv4 == 0 { + c.NodeCIDRMaskSizeIPv4 = 24 // Default to /24 per node (256 IPs) + } + // Set default pod VNIC display name if not specified + if len(c.PodVNICDisplayName) == 0 { + c.PodVNICDisplayName = "pod-vnic" + } +} + // Complete the authentication config applying defaults / overrides. func (c *AuthConfig) Complete() { if len(c.Passphrase) == 0 && len(c.PrivateKeyPassphrase) > 0 { @@ -202,6 +246,9 @@ func (c *Config) Complete() { if c.LoadBalancer != nil { c.LoadBalancer.Complete() } + if c.IPAM != nil { + c.IPAM.Complete() + } c.Auth.Complete() // Ensure backwards compatibility fields are set correctly. if len(c.CompartmentID) == 0 && len(c.Auth.CompartmentID) > 0 { diff --git a/pkg/cloudprovider/providers/oci/instances_test.go b/pkg/cloudprovider/providers/oci/instances_test.go index be6fb93a00..bb2fc45344 100644 --- a/pkg/cloudprovider/providers/oci/instances_test.go +++ b/pkg/cloudprovider/providers/oci/instances_test.go @@ -948,8 +948,12 @@ func (c *MockComputeClient) GetVnicAttachment(ctx context.Context, vnicAttachmen return nil, nil } -func (c *MockComputeClient) AttachVnic(ctx context.Context, instanceID, subnetID *string, nsgIds []*string, skipSourceDestCheck *bool) (response core.VnicAttachment, err error) { - return core.VnicAttachment{}, nil +func (c *MockComputeClient) AttachVnic(ctx context.Context, instanceID, subnetID, displayName string) (*core.VnicAttachment, error) { + return &core.VnicAttachment{}, nil +} + +func (c *MockComputeClient) DetachVnic(ctx context.Context, vnicAttachmentID string) error { + return nil } func (MockComputeClient) FindVolumeAttachment(ctx context.Context, compartmentID, volumeID string, instanceID *string) (core.VolumeAttachment, error) { diff --git a/pkg/cloudprovider/providers/oci/ipam/cidr_allocator.go b/pkg/cloudprovider/providers/oci/ipam/cidr_allocator.go new file mode 100644 index 0000000000..ed178845f6 --- /dev/null +++ b/pkg/cloudprovider/providers/oci/ipam/cidr_allocator.go @@ -0,0 +1,237 @@ +// Copyright 2024 Oracle and/or its affiliates. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package ipam + +import ( + "math/big" + "net" + "sync" + + "github.com/pkg/errors" +) + +// cidrSet implements a bitmap-based CIDR range allocator. +// It tracks which CIDR ranges have been allocated from a larger subnet CIDR. +type cidrSet struct { + sync.Mutex + + // clusterCIDR is the overall CIDR block to allocate from + clusterCIDR *net.IPNet + + // nodeMask is the mask size for individual node allocations (e.g., 24 for /24) + nodeMask int + + // allocatedCIDRs tracks which CIDRs have been allocated + // Key is the CIDR string (e.g., "10.244.1.0/24") + allocatedCIDRs map[string]bool + + // maxCIDRs is the maximum number of CIDRs that can be allocated + maxCIDRs int + + // nextCandidate is a hint for where to start searching for the next available CIDR + nextCandidate int +} + +// newCIDRSet creates a new CIDR allocator for the given cluster CIDR and node mask size. +func newCIDRSet(clusterCIDR *net.IPNet, nodeMask int) (*cidrSet, error) { + // Validate that node mask is larger than (more specific than) cluster mask + clusterMask, _ := clusterCIDR.Mask.Size() + if nodeMask <= clusterMask { + return nil, errors.Errorf("node mask (%d) must be larger than cluster mask (%d)", nodeMask, clusterMask) + } + + // Calculate maximum number of node CIDRs that can fit in the cluster CIDR + maxCIDRs := 1 << uint(nodeMask-clusterMask) + + return &cidrSet{ + clusterCIDR: clusterCIDR, + nodeMask: nodeMask, + allocatedCIDRs: make(map[string]bool), + maxCIDRs: maxCIDRs, + nextCandidate: 0, + }, nil +} + +// AllocateNext allocates the next available CIDR range. +// Returns the allocated CIDR or an error if no CIDRs are available. +func (s *cidrSet) AllocateNext() (*net.IPNet, error) { + s.Lock() + defer s.Unlock() + + if len(s.allocatedCIDRs) >= s.maxCIDRs { + return nil, errors.New("CIDR allocation exhausted: no more CIDRs available") + } + + // Search for an available CIDR starting from nextCandidate + for i := 0; i < s.maxCIDRs; i++ { + candidate := (s.nextCandidate + i) % s.maxCIDRs + cidr := s.indexToCIDR(candidate) + + if !s.allocatedCIDRs[cidr.String()] { + // Found an available CIDR + s.allocatedCIDRs[cidr.String()] = true + s.nextCandidate = (candidate + 1) % s.maxCIDRs + return cidr, nil + } + } + + return nil, errors.New("failed to find available CIDR (unexpected)") +} + +// Occupy marks a CIDR as occupied. +// This is used to register existing CIDR allocations (e.g., from nodes that already have CIDRs). +// Returns an error if the CIDR is outside the cluster CIDR or already occupied. +func (s *cidrSet) Occupy(cidr *net.IPNet) error { + s.Lock() + defer s.Unlock() + + // Verify the CIDR is within the cluster CIDR + if !s.clusterCIDR.Contains(cidr.IP) { + return errors.Errorf("CIDR %s is not within cluster CIDR %s", cidr.String(), s.clusterCIDR.String()) + } + + // Verify the CIDR has the correct mask size + maskSize, _ := cidr.Mask.Size() + if maskSize != s.nodeMask { + return errors.Errorf("CIDR %s has mask size %d, expected %d", cidr.String(), maskSize, s.nodeMask) + } + + cidrStr := cidr.String() + if s.allocatedCIDRs[cidrStr] { + return errors.Errorf("CIDR %s is already occupied", cidrStr) + } + + s.allocatedCIDRs[cidrStr] = true + return nil +} + +// Release marks a CIDR as available for reallocation. +// Returns an error if the CIDR was not previously allocated. +func (s *cidrSet) Release(cidr *net.IPNet) error { + s.Lock() + defer s.Unlock() + + cidrStr := cidr.String() + if !s.allocatedCIDRs[cidrStr] { + return errors.Errorf("CIDR %s is not currently allocated", cidrStr) + } + + delete(s.allocatedCIDRs, cidrStr) + return nil +} + +// indexToCIDR converts an index (0 to maxCIDRs-1) to a CIDR. +func (s *cidrSet) indexToCIDR(index int) *net.IPNet { + // Get the base IP of the cluster CIDR + baseIP := s.clusterCIDR.IP + + // Calculate the offset for this index + // Each index represents a block of IPs of size 2^(32-nodeMask) + clusterMask, _ := s.clusterCIDR.Mask.Size() + blockSize := 1 << uint(s.nodeMask-clusterMask) + offset := uint64(index) * uint64(blockSize) + + // Convert base IP to integer, add offset, convert back to IP + baseInt := ipToInt(baseIP) + newInt := big.NewInt(0).Add(baseInt, big.NewInt(int64(offset))) + newIP := intToIP(newInt) + + // Create the CIDR with the node mask + return &net.IPNet{ + IP: newIP, + Mask: net.CIDRMask(s.nodeMask, 32), // Assuming IPv4 for now + } +} + +// GetAllocatedCIDRs returns a list of all currently allocated CIDRs. +func (s *cidrSet) GetAllocatedCIDRs() []string { + s.Lock() + defer s.Unlock() + + cidrs := make([]string, 0, len(s.allocatedCIDRs)) + for cidr := range s.allocatedCIDRs { + cidrs = append(cidrs, cidr) + } + return cidrs +} + +// GetUsage returns allocation statistics. +func (s *cidrSet) GetUsage() (allocated, available, max int) { + s.Lock() + defer s.Unlock() + + allocated = len(s.allocatedCIDRs) + max = s.maxCIDRs + available = max - allocated + return +} + +// ipToInt converts an IP address to a big integer. +func ipToInt(ip net.IP) *big.Int { + // Ensure we're working with IPv4 + ip = ip.To4() + if ip == nil { + return big.NewInt(0) + } + + val := big.NewInt(0) + val.SetBytes(ip) + return val +} + +// intToIP converts a big integer to an IP address. +func intToIP(i *big.Int) net.IP { + // Convert to 4-byte representation (IPv4) + bytes := i.Bytes() + + // Pad to 4 bytes if necessary + if len(bytes) < 4 { + padded := make([]byte, 4) + copy(padded[4-len(bytes):], bytes) + bytes = padded + } + + return net.IP(bytes) +} + +// ParseCIDR is a helper function that parses a CIDR string and returns the IPNet. +// It's similar to net.ParseCIDR but returns only the IPNet for convenience. +func ParseCIDR(cidr string) (*net.IPNet, error) { + _, ipNet, err := net.ParseCIDR(cidr) + if err != nil { + return nil, errors.Wrapf(err, "failed to parse CIDR %s", cidr) + } + return ipNet, nil +} + +// IsCIDRAllocated checks if a specific CIDR is already allocated. +func (s *cidrSet) IsCIDRAllocated(cidr *net.IPNet) bool { + s.Lock() + defer s.Unlock() + + return s.allocatedCIDRs[cidr.String()] +} + +// GetUtilizationPercent returns the utilization percentage (0-100). +func (s *cidrSet) GetUtilizationPercent() float64 { + s.Lock() + defer s.Unlock() + + if s.maxCIDRs == 0 { + return 0 + } + + return (float64(len(s.allocatedCIDRs)) / float64(s.maxCIDRs)) * 100.0 +} diff --git a/pkg/cloudprovider/providers/oci/ipam/cloud_allocator.go b/pkg/cloudprovider/providers/oci/ipam/cloud_allocator.go new file mode 100644 index 0000000000..78cc280df3 --- /dev/null +++ b/pkg/cloudprovider/providers/oci/ipam/cloud_allocator.go @@ -0,0 +1,633 @@ +// Copyright 2024 Oracle and/or its affiliates. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package ipam + +import ( + "context" + "fmt" + "net" + "strings" + "sync" + "time" + + "github.com/oracle/oci-cloud-controller-manager/pkg/oci/client" + "github.com/pkg/errors" + "go.uber.org/zap" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/wait" + coreinformers "k8s.io/client-go/informers/core/v1" + "k8s.io/client-go/kubernetes" + "k8s.io/client-go/tools/cache" + "k8s.io/client-go/util/workqueue" +) + +const ( + // NodeNetworkUnavailableCondition is the condition type for node network availability + NodeNetworkUnavailable = v1.NodeNetworkUnavailable + + // ResyncPeriod is how often the controller reconciles all nodes + ResyncPeriod = 10 * time.Minute + + // MaxRetries is the maximum number of times a node will be retried before being dropped + MaxRetries = 15 +) + +// CloudAllocator reads pod CIDR assignments from OCI secondary VNICs +// and syncs them to Node.Spec.PodCIDR. It follows the CloudAllocator pattern +// similar to GCP's implementation. +type CloudAllocator struct { + // Kubernetes client + client kubernetes.Interface + + // OCI client interface + ociClient client.Interface + + // Node informer and lister + nodeInformer coreinformers.NodeInformer + nodeLister cache.Indexer + nodeHasSynced cache.InformerSynced + + // Work queue for processing node events + workqueue workqueue.RateLimitingInterface + + // cidrAllocators maps pod subnet ID to CIDR allocator + // Key: subnet OCID, Value: CIDR allocator for that subnet + cidrAllocators map[string]*SubnetCIDRAllocator + allocatorLock sync.RWMutex + + // Configuration + nodeCIDRMaskSize int + podSubnetIDs map[string]string // map[availabilityDomain]subnetOCID + autoAttachVNIC bool + vnicDisplayName string + + // Logger + logger *zap.SugaredLogger +} + +// NewCloudAllocator creates a new CloudAllocator instance. +func NewCloudAllocator( + client kubernetes.Interface, + ociClient client.Interface, + nodeInformer coreinformers.NodeInformer, + nodeCIDRMaskSize int, + podSubnetIDs map[string]string, + autoAttachVNIC bool, + vnicDisplayName string, + logger *zap.SugaredLogger, +) *CloudAllocator { + ca := &CloudAllocator{ + client: client, + ociClient: ociClient, + nodeInformer: nodeInformer, + nodeLister: nodeInformer.Informer().GetIndexer(), + nodeHasSynced: nodeInformer.Informer().HasSynced, + workqueue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "ipam"), + cidrAllocators: make(map[string]*SubnetCIDRAllocator), + nodeCIDRMaskSize: nodeCIDRMaskSize, + podSubnetIDs: podSubnetIDs, + autoAttachVNIC: autoAttachVNIC, + vnicDisplayName: vnicDisplayName, + logger: logger.With("controller", "ipam-cloud-allocator"), + } + + // Set up event handlers for node changes + nodeInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ + AddFunc: ca.enqueueNode, + UpdateFunc: ca.updateNode, + DeleteFunc: ca.deleteNode, + }) + + return ca +} + +// Run starts the CloudAllocator controller. +func (ca *CloudAllocator) Run(ctx context.Context, workers int) error { + defer ca.workqueue.ShutDown() + + ca.logger.Info("Starting OCI IPAM CloudAllocator controller") + + // Wait for caches to sync + ca.logger.Info("Waiting for node informer cache to sync") + if !cache.WaitForCacheSync(ctx.Done(), ca.nodeHasSynced) { + return errors.New("failed to wait for node cache to sync") + } + + // Initialize CIDR allocators for each pod subnet + if err := ca.initializeCIDRAllocators(ctx); err != nil { + return errors.Wrap(err, "failed to initialize CIDR allocators") + } + + // Reconcile existing nodes to populate CIDR allocators with existing allocations + if err := ca.reconcileExistingNodes(ctx); err != nil { + ca.logger.With("error", err).Warn("Failed to reconcile existing nodes, some CIDR conflicts may occur") + } + + // Start worker goroutines + ca.logger.With("workers", workers).Info("Starting workers") + for i := 0; i < workers; i++ { + go wait.UntilWithContext(ctx, ca.runWorker, time.Second) + } + + ca.logger.Info("OCI IPAM CloudAllocator controller started") + + // Wait until context is cancelled + <-ctx.Done() + ca.logger.Info("Shutting down OCI IPAM CloudAllocator controller") + + return nil +} + +// initializeCIDRAllocators creates CIDR allocators for each configured pod subnet. +func (ca *CloudAllocator) initializeCIDRAllocators(ctx context.Context) error { + ca.logger.Info("Initializing CIDR allocators for pod subnets") + + for ad, subnetID := range ca.podSubnetIDs { + // Get subnet details from OCI + subnet, err := ca.ociClient.Networking(nil).GetSubnet(ctx, subnetID) + if err != nil { + return errors.Wrapf(err, "failed to get subnet %s for AD %s", subnetID, ad) + } + + // Parse subnet CIDR + subnetCIDR, err := ParseCIDR(*subnet.CidrBlock) + if err != nil { + return errors.Wrapf(err, "failed to parse subnet CIDR %s", *subnet.CidrBlock) + } + + // Create CIDR allocator for this subnet + cidrSet, err := newCIDRSet(subnetCIDR, ca.nodeCIDRMaskSize) + if err != nil { + return errors.Wrapf(err, "failed to create CIDR set for subnet %s", subnetID) + } + + allocator := &SubnetCIDRAllocator{ + SubnetID: subnetID, + SubnetCIDR: subnetCIDR, + NodeMask: ca.nodeCIDRMaskSize, + CIDRSet: cidrSet, + } + + ca.allocatorLock.Lock() + ca.cidrAllocators[subnetID] = allocator + ca.allocatorLock.Unlock() + + ca.logger.With( + "availabilityDomain", ad, + "subnetID", subnetID, + "subnetCIDR", subnetCIDR.String(), + "nodeMask", ca.nodeCIDRMaskSize, + ).Info("Initialized CIDR allocator for pod subnet") + } + + return nil +} + +// reconcileExistingNodes processes all existing nodes to populate CIDR allocators +// with existing allocations, preventing conflicts. +func (ca *CloudAllocator) reconcileExistingNodes(ctx context.Context) error { + ca.logger.Info("Reconciling existing nodes with CIDR allocations") + + nodes, err := ca.client.CoreV1().Nodes().List(ctx, metav1.ListOptions{}) + if err != nil { + return errors.Wrap(err, "failed to list nodes") + } + + for _, node := range nodes.Items { + if node.Spec.PodCIDR == "" { + continue + } + + // Parse the node's pod CIDR + _, podCIDR, err := net.ParseCIDR(node.Spec.PodCIDR) + if err != nil { + ca.logger.With("node", node.Name, "podCIDR", node.Spec.PodCIDR, "error", err).Warn("Failed to parse existing pod CIDR") + continue + } + + // Find which subnet this CIDR belongs to and mark it as occupied + if err := ca.occupyExistingCIDR(ctx, &node, podCIDR); err != nil { + ca.logger.With("node", node.Name, "podCIDR", podCIDR.String(), "error", err).Warn("Failed to occupy existing CIDR") + } + } + + ca.logger.With("nodeCount", len(nodes.Items)).Info("Reconciliation of existing nodes complete") + return nil +} + +// occupyExistingCIDR marks an existing CIDR as occupied in the appropriate allocator. +func (ca *CloudAllocator) occupyExistingCIDR(ctx context.Context, node *v1.Node, cidr *net.IPNet) error { + // Get availability domain from node labels + availabilityDomain, ok := node.Labels["topology.kubernetes.io/zone"] + if !ok { + // Fall back to deprecated label + availabilityDomain, ok = node.Labels["failure-domain.beta.kubernetes.io/zone"] + if !ok { + return errors.New("node does not have availability domain label (topology.kubernetes.io/zone)") + } + } + + // Get the pod subnet for this node's availability domain + // Try exact match first + subnetID, ok := ca.podSubnetIDs[availabilityDomain] + if !ok { + // If not found, try with realm prefix (format: "realm:AD-NAME") + for configAD, configSubnetID := range ca.podSubnetIDs { + if parts := strings.Split(configAD, ":"); len(parts) == 2 && parts[1] == availabilityDomain { + subnetID = configSubnetID + ok = true + break + } + } + if !ok { + return errors.Errorf("no pod subnet configured for availability domain %s", availabilityDomain) + } + } + + // Get the allocator for this subnet + ca.allocatorLock.RLock() + allocator, ok := ca.cidrAllocators[subnetID] + ca.allocatorLock.RUnlock() + + if !ok { + return errors.Errorf("no CIDR allocator found for subnet %s", subnetID) + } + + // Mark the CIDR as occupied + if err := allocator.CIDRSet.Occupy(cidr); err != nil { + return errors.Wrap(err, "failed to occupy CIDR") + } + + ca.logger.With("node", node.Name, "podCIDR", cidr.String(), "subnetID", subnetID).Debug("Marked existing pod CIDR as occupied") + return nil +} + +// runWorker is a long-running function that will continually call processNextWorkItem +// to read and process a message on the workqueue. +func (ca *CloudAllocator) runWorker(ctx context.Context) { + for ca.processNextWorkItem(ctx) { + } +} + +// processNextWorkItem reads a single work item off the workqueue and processes it. +func (ca *CloudAllocator) processNextWorkItem(ctx context.Context) bool { + obj, shutdown := ca.workqueue.Get() + if shutdown { + return false + } + + err := func(obj interface{}) error { + defer ca.workqueue.Done(obj) + + key, ok := obj.(string) + if !ok { + ca.workqueue.Forget(obj) + ca.logger.With("object", obj).Warn("Expected string in workqueue but got something else") + return nil + } + + // Process the node + if err := ca.processNode(ctx, key); err != nil { + // Requeue the item if processing failed + if ca.workqueue.NumRequeues(key) < MaxRetries { + ca.workqueue.AddRateLimited(key) + return errors.Wrapf(err, "error processing node %s, requeuing", key) + } + + // Max retries reached, drop the item + ca.workqueue.Forget(key) + ca.logger.With("node", key, "error", err).Error("Dropping node after max retries") + return nil + } + + // Successfully processed, forget the item + ca.workqueue.Forget(key) + return nil + }(obj) + + if err != nil { + ca.logger.With("error", err).Error("Error processing work item") + } + + return true +} + +// processNode processes a single node, allocating a pod CIDR if needed. +func (ca *CloudAllocator) processNode(ctx context.Context, key string) error { + // Get the node from the cache + obj, exists, err := ca.nodeLister.GetByKey(key) + if err != nil { + return errors.Wrapf(err, "failed to get node %s from cache", key) + } + + if !exists { + // Node was deleted, nothing to do + ca.logger.With("node", key).Debug("Node no longer exists") + return nil + } + + node := obj.(*v1.Node) + + // Check if node already has a pod CIDR + if node.Spec.PodCIDR != "" { + ca.logger.With("node", node.Name, "podCIDR", node.Spec.PodCIDR).Debug("Node already has pod CIDR") + return nil + } + + ca.logger.With("node", node.Name).Info("Processing node for CIDR allocation") + + // Allocate a pod CIDR for this node + podCIDR, subnetID, err := ca.allocatePodCIDRForNode(ctx, node) + if err != nil { + return errors.Wrapf(err, "failed to allocate pod CIDR for node %s", node.Name) + } + + // Update the node with the allocated pod CIDR + if err := ca.updateNodePodCIDR(ctx, node, podCIDR); err != nil { + // Failed to update node, release the CIDR + ca.releaseCIDR(subnetID, podCIDR) + return errors.Wrapf(err, "failed to update node %s with pod CIDR", node.Name) + } + + ca.logger.With("node", node.Name, "podCIDR", podCIDR).Info("Successfully allocated and assigned pod CIDR to node") + return nil +} + +// allocatePodCIDRForNode allocates a pod CIDR for the given node from the appropriate subnet. +func (ca *CloudAllocator) allocatePodCIDRForNode(ctx context.Context, node *v1.Node) (string, string, error) { + // Get availability domain from node labels (set by kubelet/cloud provider) + // Standard Kubernetes topology label + availabilityDomain, ok := node.Labels["topology.kubernetes.io/zone"] + if !ok { + // Fall back to deprecated label + availabilityDomain, ok = node.Labels["failure-domain.beta.kubernetes.io/zone"] + if !ok { + return "", "", errors.New("node does not have availability domain label (topology.kubernetes.io/zone), will retry") + } + } + + ca.logger.With( + "node", node.Name, + "availabilityDomain", availabilityDomain, + ).Debug("Processing node for pod CIDR allocation") + + // Get the pod subnet ID for this availability domain + // Try exact match first + subnetID, ok := ca.podSubnetIDs[availabilityDomain] + if !ok { + // If not found, try with realm prefix (format: "realm:AD-NAME") + // This handles cases where config uses full format but label has short format + for configAD, configSubnetID := range ca.podSubnetIDs { + // Check if the config AD ends with the node's AD (after the colon) + if parts := strings.Split(configAD, ":"); len(parts) == 2 && parts[1] == availabilityDomain { + subnetID = configSubnetID + ok = true + ca.logger.With( + "configuredAD", configAD, + "nodeAD", availabilityDomain, + "subnetID", subnetID, + ).Debug("Matched availability domain with realm prefix") + break + } + } + if !ok { + return "", "", errors.Errorf("no pod subnet configured for availability domain %s", availabilityDomain) + } + } + + // Get the CIDR allocator for this subnet + ca.allocatorLock.RLock() + allocator, ok := ca.cidrAllocators[subnetID] + ca.allocatorLock.RUnlock() + + if !ok { + return "", "", errors.Errorf("no CIDR allocator found for subnet %s", subnetID) + } + + // Allocate next available CIDR + cidr, err := allocator.CIDRSet.AllocateNext() + if err != nil { + return "", "", errors.Wrap(err, "failed to allocate CIDR from subnet") + } + + ca.logger.With( + "node", node.Name, + "availabilityDomain", availabilityDomain, + "subnetID", subnetID, + "allocatedCIDR", cidr.String(), + ).Info("Allocated pod CIDR for node") + + return cidr.String(), subnetID, nil +} + +// updateNodePodCIDR updates the node's Spec.PodCIDR field and sets the NodeNetworkUnavailable condition to False. +func (ca *CloudAllocator) updateNodePodCIDR(ctx context.Context, node *v1.Node, podCIDR string) error { + // Create a patch to update the node + patch := fmt.Sprintf(`{"spec":{"podCIDR":"%s","podCIDRs":["%s"]}}`, podCIDR, podCIDR) + + _, err := ca.client.CoreV1().Nodes().Patch( + ctx, + node.Name, + types.StrategicMergePatchType, + []byte(patch), + metav1.PatchOptions{}, + ) + + if err != nil { + return errors.Wrap(err, "failed to patch node with pod CIDR") + } + + // Update the NodeNetworkUnavailable condition to False + // This signals that the node's network is properly configured + if err := ca.updateNodeNetworkCondition(ctx, node); err != nil { + ca.logger.With("node", node.Name, "error", err).Warn("Failed to update node network condition") + // Don't return error here as the CIDR was successfully assigned + } + + return nil +} + +// updateNodeNetworkCondition sets the NodeNetworkUnavailable condition to False. +func (ca *CloudAllocator) updateNodeNetworkCondition(ctx context.Context, node *v1.Node) error { + // Get the latest version of the node + currentNode, err := ca.client.CoreV1().Nodes().Get(ctx, node.Name, metav1.GetOptions{}) + if err != nil { + return errors.Wrap(err, "failed to get node") + } + + // Check if condition already exists and is False + for _, condition := range currentNode.Status.Conditions { + if condition.Type == NodeNetworkUnavailable && condition.Status == v1.ConditionFalse { + // Already set correctly + return nil + } + } + + // Create the condition patch + now := metav1.Now() + condition := v1.NodeCondition{ + Type: NodeNetworkUnavailable, + Status: v1.ConditionFalse, + Reason: "RouteCreated", + Message: "OCI IPAM allocated pod CIDR", + LastTransitionTime: now, + LastHeartbeatTime: now, + } + + // Find existing condition index + conditionIndex := -1 + for i, c := range currentNode.Status.Conditions { + if c.Type == NodeNetworkUnavailable { + conditionIndex = i + break + } + } + + if conditionIndex >= 0 { + // Update existing condition + patch := fmt.Sprintf(`{"status":{"conditions":[{"type":"%s","status":"%s","reason":"%s","message":"%s","lastTransitionTime":"%s","lastHeartbeatTime":"%s"}]}}`, + condition.Type, condition.Status, condition.Reason, condition.Message, + condition.LastTransitionTime.Format(time.RFC3339), condition.LastHeartbeatTime.Format(time.RFC3339)) + + _, err = ca.client.CoreV1().Nodes().PatchStatus(ctx, node.Name, []byte(patch)) + } else { + // Append new condition + currentNode.Status.Conditions = append(currentNode.Status.Conditions, condition) + _, err = ca.client.CoreV1().Nodes().UpdateStatus(ctx, currentNode, metav1.UpdateOptions{}) + } + + return err +} + +// releaseCIDR releases a CIDR back to the allocator pool. +func (ca *CloudAllocator) releaseCIDR(subnetID string, podCIDR string) { + ca.allocatorLock.RLock() + allocator, ok := ca.cidrAllocators[subnetID] + ca.allocatorLock.RUnlock() + + if !ok { + ca.logger.With("subnetID", subnetID).Warn("No allocator found for subnet, cannot release CIDR") + return + } + + cidr, err := ParseCIDR(podCIDR) + if err != nil { + ca.logger.With("podCIDR", podCIDR, "error", err).Warn("Failed to parse CIDR for release") + return + } + + if err := allocator.CIDRSet.Release(cidr); err != nil { + ca.logger.With("podCIDR", podCIDR, "error", err).Warn("Failed to release CIDR") + return + } + + ca.logger.With("podCIDR", podCIDR, "subnetID", subnetID).Debug("Released CIDR back to pool") +} + +// enqueueNode adds a node to the workqueue. +func (ca *CloudAllocator) enqueueNode(obj interface{}) { + key, err := cache.MetaNamespaceKeyFunc(obj) + if err != nil { + ca.logger.With("error", err).Error("Failed to get key for node") + return + } + ca.workqueue.Add(key) +} + +// updateNode handles node update events. +func (ca *CloudAllocator) updateNode(old, new interface{}) { + oldNode := old.(*v1.Node) + newNode := new.(*v1.Node) + + // Only process if the node doesn't have a pod CIDR yet + if newNode.Spec.PodCIDR == "" && oldNode.Spec.PodCIDR == "" { + ca.enqueueNode(new) + } +} + +// deleteNode handles node deletion events. +func (ca *CloudAllocator) deleteNode(obj interface{}) { + node, ok := obj.(*v1.Node) + if !ok { + tombstone, ok := obj.(cache.DeletedFinalStateUnknown) + if !ok { + ca.logger.With("object", obj).Error("Error decoding object tombstone") + return + } + node, ok = tombstone.Obj.(*v1.Node) + if !ok { + ca.logger.With("object", obj).Error("Error decoding object tombstone, invalid type") + return + } + } + + // Release the node's CIDR if it had one + if node.Spec.PodCIDR != "" { + ca.logger.With("node", node.Name, "podCIDR", node.Spec.PodCIDR).Info("Node deleted, releasing CIDR") + + // Get availability domain from node labels + availabilityDomain, ok := node.Labels["topology.kubernetes.io/zone"] + if !ok { + // Fall back to deprecated label + availabilityDomain, ok = node.Labels["failure-domain.beta.kubernetes.io/zone"] + if !ok { + ca.logger.With("node", node.Name).Warn("Node does not have availability domain label, cannot release CIDR") + return + } + } + + // Get the pod subnet for this availability domain + subnetID, ok := ca.podSubnetIDs[availabilityDomain] + if !ok { + // Try with realm prefix + for configAD, configSubnetID := range ca.podSubnetIDs { + if parts := strings.Split(configAD, ":"); len(parts) == 2 && parts[1] == availabilityDomain { + subnetID = configSubnetID + ok = true + break + } + } + } + + if ok { + ca.releaseCIDR(subnetID, node.Spec.PodCIDR) + } else { + ca.logger.With("node", node.Name, "availabilityDomain", availabilityDomain).Warn("No pod subnet configured for availability domain, cannot release CIDR") + } + } +} + +// getInstanceIDFromProviderID extracts the instance OCID from a Kubernetes provider ID. +// Provider ID format: :// +func getInstanceIDFromProviderID(providerID string) (string, error) { + if providerID == "" { + return "", errors.New("provider ID is empty") + } + + // Expected format: oci:// + const prefix = "oci://" + if len(providerID) <= len(prefix) { + return "", errors.Errorf("invalid provider ID format: %s", providerID) + } + + instanceID := providerID[len(prefix):] + if instanceID == "" { + return "", errors.Errorf("instance ID is empty in provider ID: %s", providerID) + } + + return instanceID, nil +} diff --git a/pkg/cloudprovider/providers/oci/ipam/types.go b/pkg/cloudprovider/providers/oci/ipam/types.go new file mode 100644 index 0000000000..42f5e10acc --- /dev/null +++ b/pkg/cloudprovider/providers/oci/ipam/types.go @@ -0,0 +1,53 @@ +// Copyright 2024 Oracle and/or its affiliates. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package ipam + +import ( + "net" +) + +// SubnetCIDRAllocator manages CIDR allocations within a specific OCI subnet. +// It tracks which CIDR ranges have been allocated to nodes to prevent overlaps. +type SubnetCIDRAllocator struct { + // SubnetID is the OCID of the OCI subnet + SubnetID string + + // SubnetCIDR is the CIDR block of the subnet + SubnetCIDR *net.IPNet + + // NodeMask is the mask size for individual node CIDRs (e.g., 24 for /24) + NodeMask int + + // CIDRSet manages the actual CIDR allocation bitmap + CIDRSet *cidrSet +} + +// NodeCIDRAllocation represents a CIDR allocation for a specific node. +type NodeCIDRAllocation struct { + // NodeName is the Kubernetes node name + NodeName string + + // InstanceID is the OCI instance OCID + InstanceID string + + // PodCIDR is the allocated pod CIDR for this node + PodCIDR *net.IPNet + + // SubnetID is the pod subnet OCID where the node's pod VNIC is attached + SubnetID string + + // VnicID is the OCID of the pod VNIC attached to this node + VnicID string +} diff --git a/pkg/cloudprovider/providers/oci/routes.go b/pkg/cloudprovider/providers/oci/routes.go new file mode 100644 index 0000000000..2af4e17b20 --- /dev/null +++ b/pkg/cloudprovider/providers/oci/routes.go @@ -0,0 +1,259 @@ +// Copyright 2024 Oracle and/or its affiliates. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package oci + +import ( + "context" + "strings" + + "github.com/oracle/oci-go-sdk/v65/core" + "github.com/pkg/errors" + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/types" + cloudprovider "k8s.io/cloud-provider" +) + +// routes implements the cloudprovider.Routes interface for OCI. +// Note: OCI uses native subnet routing via VNICs, so this implementation +// primarily validates that routing is correctly configured rather than +// managing explicit route table entries. When a secondary VNIC is attached +// to a pod subnet, OCI automatically routes traffic destined for that +// subnet to the instance. +type routes struct { + cp *CloudProvider +} + +// ListRoutes lists all managed routes for the specified cluster. +// In the OCI implementation, routes are implicitly created through +// secondary VNIC attachments in pod subnets. This method returns +// Route objects representing those VNIC-based routes. +func (r *routes) ListRoutes(ctx context.Context, clusterName string) ([]*cloudprovider.Route, error) { + r.cp.logger.With("clusterName", clusterName).Debug("Listing routes") + + // Get all nodes in the cluster + nodes, err := r.cp.NodeLister.List(labels.Everything()) + if err != nil { + return nil, errors.Wrap(err, "failed to list nodes") + } + + var routeList []*cloudprovider.Route + + // For each node with a PodCIDR, verify secondary VNIC exists + for _, node := range nodes { + if node.Spec.PodCIDR == "" { + // Node doesn't have PodCIDR yet, skip + continue + } + + // Get the instance ID from provider ID + instanceID, err := MapProviderIDToResourceID(node.Spec.ProviderID) + if err != nil { + r.cp.logger.With("node", node.Name, "error", err).Warn("Failed to map provider ID to instance ID") + continue + } + + // Get compartment ID for the node + compartmentID, err := r.cp.getCompartmentIDByInstanceID(instanceID) + if err != nil { + r.cp.logger.With("node", node.Name, "error", err).Warn("Failed to get compartment ID") + continue + } + + // Check if secondary VNIC exists in pod subnet + podVNIC, err := r.getPodVNICForNode(ctx, instanceID, compartmentID, node) + if err != nil { + r.cp.logger.With("node", node.Name, "error", err).Warn("Failed to get pod VNIC for node") + continue + } + + if podVNIC == nil { + r.cp.logger.With("node", node.Name).Warn("No pod VNIC found for node with PodCIDR") + continue + } + + // Create a Route object representing the VNIC-based route + route := &cloudprovider.Route{ + Name: node.Name, + TargetNode: types.NodeName(node.Name), + DestinationCIDR: node.Spec.PodCIDR, + } + routeList = append(routeList, route) + } + + r.cp.logger.With("routeCount", len(routeList)).Debug("Listed routes") + return routeList, nil +} + +// CreateRoute creates a new route for the specified cluster. +// In the OCI implementation, routes are created implicitly when secondary +// VNICs are attached to pod subnets. This method validates that the +// target node has a secondary VNIC in the correct pod subnet. If +// autoAttachPodVNIC is enabled and the VNIC doesn't exist, it will +// attach one. +func (r *routes) CreateRoute(ctx context.Context, clusterName string, nameHint string, route *cloudprovider.Route) error { + r.cp.logger.With( + "clusterName", clusterName, + "nameHint", nameHint, + "targetNode", route.TargetNode, + "destinationCIDR", route.DestinationCIDR, + ).Debug("Creating route") + + // Get the target node + node, err := r.cp.NodeLister.Get(string(route.TargetNode)) + if err != nil { + return errors.Wrapf(err, "failed to get node %s", route.TargetNode) + } + + // Get the instance ID from provider ID + instanceID, err := MapProviderIDToResourceID(node.Spec.ProviderID) + if err != nil { + return errors.Wrap(err, "failed to map provider ID to instance ID") + } + + // Get compartment ID for the node + compartmentID, err := r.cp.getCompartmentIDByInstanceID(instanceID) + if err != nil { + return errors.Wrap(err, "failed to get compartment ID") + } + + // Check if secondary VNIC exists in pod subnet + podVNIC, err := r.getPodVNICForNode(ctx, instanceID, compartmentID, node) + if err != nil { + return errors.Wrap(err, "failed to get pod VNIC for node") + } + + if podVNIC != nil { + // VNIC already exists, routing is automatically handled by OCI + r.cp.logger.With( + "node", node.Name, + "vnicID", *podVNIC.Id, + ).Info("Pod VNIC already exists, routing is handled natively by OCI VCN") + return nil + } + + // VNIC doesn't exist + if !r.cp.config.IPAM.AutoAttachPodVNIC { + // Auto-attach is disabled, return error + return errors.Errorf("no pod VNIC found for node %s and autoAttachPodVNIC is disabled", node.Name) + } + + // Auto-attach is enabled, attach a secondary VNIC + r.cp.logger.With("node", node.Name).Info("Attaching pod VNIC to node") + + // Get availability domain from node labels + availabilityDomain, ok := node.Labels["topology.kubernetes.io/zone"] + if !ok { + // Fall back to deprecated label + availabilityDomain, ok = node.Labels["failure-domain.beta.kubernetes.io/zone"] + if !ok { + return errors.New("node does not have availability domain label (topology.kubernetes.io/zone)") + } + } + + // Get the pod subnet ID for this availability domain + // Try exact match first + podSubnetID, ok := r.cp.config.IPAM.PodSubnetIDs[availabilityDomain] + if !ok { + // Try with realm prefix + for configAD, configSubnetID := range r.cp.config.IPAM.PodSubnetIDs { + if parts := strings.Split(configAD, ":"); len(parts) == 2 && parts[1] == availabilityDomain { + podSubnetID = configSubnetID + ok = true + break + } + } + if !ok { + return errors.Errorf("no pod subnet configured for availability domain %s", availabilityDomain) + } + } + + // Attach the VNIC + _, err = r.cp.client.Compute().AttachVnic(ctx, instanceID, podSubnetID, r.cp.config.IPAM.PodVNICDisplayName) + if err != nil { + return errors.Wrap(err, "failed to attach pod VNIC") + } + + r.cp.logger.With("node", node.Name, "subnetID", podSubnetID).Info("Successfully attached pod VNIC to node") + return nil +} + +// DeleteRoute deletes the specified route. +// In the OCI implementation, we don't actually delete routes since they are +// managed implicitly through VNIC attachments. Detaching a VNIC could +// disrupt networking, so this is a no-op. The IPAM controller is responsible +// for managing VNIC lifecycle. +func (r *routes) DeleteRoute(ctx context.Context, clusterName string, route *cloudprovider.Route) error { + r.cp.logger.With( + "clusterName", clusterName, + "targetNode", route.TargetNode, + "destinationCIDR", route.DestinationCIDR, + ).Debug("Delete route requested (no-op for OCI)") + + // Note: We don't detach VNICs as it could disrupt pod networking. + // VNIC cleanup should be handled separately if needed. + return nil +} + +// getPodVNICForNode finds the secondary VNIC in a pod subnet for the given node. +// Returns nil if no pod VNIC is found. +func (r *routes) getPodVNICForNode(ctx context.Context, instanceID, compartmentID string, node *v1.Node) (*core.Vnic, error) { + // Get availability domain from node labels (set by kubelet/cloud provider) + // Standard Kubernetes topology label + availabilityDomain, ok := node.Labels["topology.kubernetes.io/zone"] + if !ok { + // Fall back to deprecated label + availabilityDomain, ok = node.Labels["failure-domain.beta.kubernetes.io/zone"] + if !ok { + return nil, errors.New("node does not have availability domain label (topology.kubernetes.io/zone)") + } + } + + // Get the pod subnet ID for this availability domain + // Try exact match first + podSubnetID, ok := r.cp.config.IPAM.PodSubnetIDs[availabilityDomain] + if !ok { + // If not found, try with realm prefix (format: "realm:AD-NAME") + // This handles cases where config uses full format but label has short format + for configAD, configSubnetID := range r.cp.config.IPAM.PodSubnetIDs { + // Check if the config AD ends with the node's AD (after the colon) + if parts := strings.Split(configAD, ":"); len(parts) == 2 && parts[1] == availabilityDomain { + podSubnetID = configSubnetID + ok = true + break + } + } + if !ok { + // No pod subnet configured for this AD + return nil, errors.Errorf("no pod subnet configured for availability domain %s", availabilityDomain) + } + } + + // Get secondary VNICs for the instance + secondaryVNICs, err := r.cp.client.Compute().GetSecondaryVNICsForInstance(ctx, compartmentID, instanceID) + if err != nil { + return nil, errors.Wrap(err, "failed to get secondary VNICs") + } + + // Look for a VNIC in the pod subnet + for _, vnic := range secondaryVNICs { + if vnic.SubnetId != nil && *vnic.SubnetId == podSubnetID { + return vnic, nil + } + } + + // No pod VNIC found + return nil, nil +} diff --git a/pkg/csi/driver/bv_controller_test.go b/pkg/csi/driver/bv_controller_test.go index 0e8b584dfc..aa3284e4b5 100644 --- a/pkg/csi/driver/bv_controller_test.go +++ b/pkg/csi/driver/bv_controller_test.go @@ -660,8 +660,12 @@ func (c *MockComputeClient) GetVnicAttachment(ctx context.Context, vnicAttachmen return nil, nil } -func (c *MockComputeClient) AttachVnic(ctx context.Context, instanceID, subnetID *string, nsgIds []*string, skipSourceDestCheck *bool) (response core.VnicAttachment, err error) { - return core.VnicAttachment{}, nil +func (c *MockComputeClient) AttachVnic(ctx context.Context, instanceID, subnetID, displayName string) (*core.VnicAttachment, error) { + return &core.VnicAttachment{}, nil +} + +func (c *MockComputeClient) DetachVnic(ctx context.Context, vnicAttachmentID string) error { + return nil } func (c *MockComputeClient) FindVolumeAttachment(ctx context.Context, compartmentID, volumeID string, instanceID *string) (core.VolumeAttachment, error) { diff --git a/pkg/oci/client/client.go b/pkg/oci/client/client.go index dd362ad66a..8205bdcc88 100644 --- a/pkg/oci/client/client.go +++ b/pkg/oci/client/client.go @@ -67,6 +67,8 @@ type computeClient interface { GetInstance(ctx context.Context, request core.GetInstanceRequest) (response core.GetInstanceResponse, err error) ListInstances(ctx context.Context, request core.ListInstancesRequest) (response core.ListInstancesResponse, err error) ListVnicAttachments(ctx context.Context, request core.ListVnicAttachmentsRequest) (response core.ListVnicAttachmentsResponse, err error) + AttachVnic(ctx context.Context, request core.AttachVnicRequest) (response core.AttachVnicResponse, err error) + DetachVnic(ctx context.Context, request core.DetachVnicRequest) (response core.DetachVnicResponse, err error) GetVolumeAttachment(ctx context.Context, request core.GetVolumeAttachmentRequest) (response core.GetVolumeAttachmentResponse, err error) ListVolumeAttachments(ctx context.Context, request core.ListVolumeAttachmentsRequest) (response core.ListVolumeAttachmentsResponse, err error) diff --git a/pkg/oci/client/client_test.go b/pkg/oci/client/client_test.go index b11454f848..c56fe1ce7f 100644 --- a/pkg/oci/client/client_test.go +++ b/pkg/oci/client/client_test.go @@ -166,6 +166,10 @@ func (c *mockComputeClient) AttachVnic(ctx context.Context, request core.AttachV return core.AttachVnicResponse{}, nil } +func (c *mockComputeClient) DetachVnic(ctx context.Context, request core.DetachVnicRequest) (response core.DetachVnicResponse, err error) { + return core.DetachVnicResponse{}, nil +} + func (c *mockComputeClient) GetVolumeAttachment(ctx context.Context, request core.GetVolumeAttachmentRequest) (response core.GetVolumeAttachmentResponse, err error) { return core.GetVolumeAttachmentResponse{}, nil } diff --git a/pkg/oci/client/compute.go b/pkg/oci/client/compute.go index 99e3993892..1eb3ce8b70 100644 --- a/pkg/oci/client/compute.go +++ b/pkg/oci/client/compute.go @@ -36,6 +36,12 @@ type ComputeInterface interface { GetSecondaryVNICsForInstance(ctx context.Context, compartmentID, instanceID string) ([]*core.Vnic, error) + // AttachVnic attaches a secondary VNIC to an instance + AttachVnic(ctx context.Context, instanceID, subnetID, displayName string) (*core.VnicAttachment, error) + + // DetachVnic detaches a VNIC from an instance + DetachVnic(ctx context.Context, vnicAttachmentID string) error + VolumeAttachmentInterface } @@ -335,3 +341,61 @@ func getNonTerminalInstances(instances []core.Instance) []core.Instance { } return result } + +// AttachVnic attaches a secondary VNIC to an instance in the specified subnet. +func (c *client) AttachVnic(ctx context.Context, instanceID, subnetID, displayName string) (*core.VnicAttachment, error) { + logger := c.logger.With("instanceID", instanceID, "subnetID", subnetID) + logger.Debug("Attaching VNIC to instance") + + if !c.rateLimiter.Writer.TryAccept() { + return nil, RateLimitError(true, "AttachVnic") + } + + // Create the VNIC attachment + resp, err := c.compute.AttachVnic(ctx, core.AttachVnicRequest{ + AttachVnicDetails: core.AttachVnicDetails{ + InstanceId: &instanceID, + DisplayName: &displayName, + CreateVnicDetails: &core.CreateVnicDetails{ + SubnetId: &subnetID, + DisplayName: &displayName, + // Add freeform tag to identify CCM-managed VNICs + FreeformTags: map[string]string{ + "oci-ccm-managed": "pod-vnic", + }, + }, + }, + RequestMetadata: c.requestMetadata, + }) + incRequestCounter(err, createVerb, vnicAttachmentResource) + + if err != nil { + return nil, errors.WithStack(err) + } + + logger.With("vnicAttachmentID", *resp.Id).Info("Successfully initiated VNIC attachment") + return &resp.VnicAttachment, nil +} + +// DetachVnic detaches a VNIC from an instance. +func (c *client) DetachVnic(ctx context.Context, vnicAttachmentID string) error { + logger := c.logger.With("vnicAttachmentID", vnicAttachmentID) + logger.Debug("Detaching VNIC from instance") + + if !c.rateLimiter.Writer.TryAccept() { + return RateLimitError(true, "DetachVnic") + } + + _, err := c.compute.DetachVnic(ctx, core.DetachVnicRequest{ + VnicAttachmentId: &vnicAttachmentID, + RequestMetadata: c.requestMetadata, + }) + incRequestCounter(err, deleteVerb, vnicAttachmentResource) + + if err != nil { + return errors.WithStack(err) + } + + logger.Info("Successfully initiated VNIC detachment") + return nil +} diff --git a/pkg/volume/provisioner/block/block_test.go b/pkg/volume/provisioner/block/block_test.go index 38d83ab171..c899b5820f 100644 --- a/pkg/volume/provisioner/block/block_test.go +++ b/pkg/volume/provisioner/block/block_test.go @@ -264,8 +264,12 @@ func (c *MockComputeClient) GetVnicAttachment(ctx context.Context, vnicAttachmen return nil, nil } -func (c *MockComputeClient) AttachVnic(ctx context.Context, instanceID, subnetID *string, nsgIds []*string, skipSourceDestCheck *bool) (response core.VnicAttachment, err error) { - return core.VnicAttachment{}, nil +func (c *MockComputeClient) AttachVnic(ctx context.Context, instanceID, subnetID, displayName string) (*core.VnicAttachment, error) { + return &core.VnicAttachment{}, nil +} + +func (c *MockComputeClient) DetachVnic(ctx context.Context, vnicAttachmentID string) error { + return nil } func (c *MockComputeClient) FindVolumeAttachment(ctx context.Context, compartmentID, volumeID string, instanceID *string) (core.VolumeAttachment, error) { diff --git a/pkg/volume/provisioner/fss/fss_test.go b/pkg/volume/provisioner/fss/fss_test.go index e9193f1f07..b01198be79 100644 --- a/pkg/volume/provisioner/fss/fss_test.go +++ b/pkg/volume/provisioner/fss/fss_test.go @@ -260,8 +260,12 @@ func (c *MockComputeClient) GetVnicAttachment(ctx context.Context, vnicAttachmen return nil, nil } -func (c *MockComputeClient) AttachVnic(ctx context.Context, instanceID, subnetID *string, nsgIds []*string, skipSourceDestCheck *bool) (response core.VnicAttachment, err error) { - return core.VnicAttachment{}, nil +func (c *MockComputeClient) AttachVnic(ctx context.Context, instanceID, subnetID, displayName string) (*core.VnicAttachment, error) { + return &core.VnicAttachment{}, nil +} + +func (c *MockComputeClient) DetachVnic(ctx context.Context, vnicAttachmentID string) error { + return nil } func (c *MockComputeClient) FindVolumeAttachment(ctx context.Context, compartmentID, volumeID string, instanceID *string) (core.VolumeAttachment, error) { From 362a5dc692f0c03d7438c4ed1705924e8fe26007 Mon Sep 17 00:00:00 2001 From: bob Date: Tue, 10 Mar 2026 17:00:43 +0100 Subject: [PATCH 5/5] External-ccm: Support key_file in auth config Allow specifying a private key file path (key_file) as an alternative to inlining the key directly. The file is read during config completion and the two options are mutually exclusive. --- .../providers/oci/config/config.go | 34 +++++++++++++++---- .../providers/oci/config/config_validate.go | 7 +++- 2 files changed, 33 insertions(+), 8 deletions(-) diff --git a/pkg/cloudprovider/providers/oci/config/config.go b/pkg/cloudprovider/providers/oci/config/config.go index f70f540d58..6206159560 100644 --- a/pkg/cloudprovider/providers/oci/config/config.go +++ b/pkg/cloudprovider/providers/oci/config/config.go @@ -16,10 +16,11 @@ package config import ( "fmt" - "github.com/oracle/oci-cloud-controller-manager/pkg/oci/instance/metadata" "io" "os" + "github.com/oracle/oci-cloud-controller-manager/pkg/oci/instance/metadata" + "github.com/oracle/oci-go-sdk/v65/common" "github.com/oracle/oci-go-sdk/v65/common/auth" "github.com/pkg/errors" @@ -30,12 +31,13 @@ import ( // AuthConfig holds the configuration required for communicating with the OCI // API. type AuthConfig struct { - Region string `yaml:"region"` - TenancyID string `yaml:"tenancy"` - UserID string `yaml:"user"` - PrivateKey string `yaml:"key"` - Fingerprint string `yaml:"fingerprint"` - Passphrase string `yaml:"passphrase"` + Region string `yaml:"region"` + TenancyID string `yaml:"tenancy"` + UserID string `yaml:"user"` + PrivateKey string `yaml:"key"` + PrivateKeyFile string `yaml:"key_file"` + Fingerprint string `yaml:"fingerprint"` + Passphrase string `yaml:"passphrase"` // Used by the flex driver for OCID expansion. This should be moved to top level // as it doesn't strictly relate to OCI authentication. @@ -225,6 +227,16 @@ func (c *AuthConfig) Complete() { zap.S().Warn("cloud-provider config: auth.key_passphrase is DEPRECIATED and will be removed in a later release. Please set auth.passphrase instead.") c.Passphrase = c.PrivateKeyPassphrase } + // Resolve PrivateKeyFile into PrivateKey when PrivateKey is not set directly. + if c.PrivateKey == "" && c.PrivateKeyFile != "" { + keyBytes, err := os.ReadFile(c.PrivateKeyFile) + if err != nil { + zap.S().Errorf("cloud-provider config: failed to read private key file %q: %v", c.PrivateKeyFile, err) + } else { + c.PrivateKey = string(keyBytes) + c.PrivateKeyFile = "" + } + } if c.Region == "" || c.CompartmentID == "" { meta, err := c.metadataSvc.Get() if err != nil { @@ -345,6 +357,14 @@ func NewConfigurationProvider(cfg *Config) (common.ConfigurationProvider, error) return cp, nil } + if ociConfigFile := os.Getenv("OCI_CONFIG_FILE"); ociConfigFile != "" { + cp, err := common.ConfigurationProviderFromFile(ociConfigFile, cfg.Auth.Passphrase) + if err != nil { + return nil, fmt.Errorf("failed to load OCI config from %s: %w", ociConfigFile, err) + } + return cp, nil + } + conf = common.NewRawConfigurationProvider( cfg.Auth.TenancyID, cfg.Auth.UserID, diff --git a/pkg/cloudprovider/providers/oci/config/config_validate.go b/pkg/cloudprovider/providers/oci/config/config_validate.go index 8e59fcb338..acf596a8e3 100644 --- a/pkg/cloudprovider/providers/oci/config/config_validate.go +++ b/pkg/cloudprovider/providers/oci/config/config_validate.go @@ -15,6 +15,8 @@ package config import ( + "os" + "github.com/pkg/errors" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation/field" @@ -26,6 +28,9 @@ func validateAuthConfig(c *AuthConfig, fldPath *field.Path) field.ErrorList { if c == nil { return append(allErrs, field.Required(fldPath, "")) } + if c.PrivateKey != "" && c.PrivateKeyFile != "" { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("key_file"), "key_file cannot be set when key is already provided")) + } checkFields := map[string]string{ "region": c.Region, "tenancy": c.TenancyID, @@ -81,7 +86,7 @@ func ValidateConfig(c *Config) field.ErrorList { allErrs = append(allErrs, field.Forbidden(field.NewPath("useWorkloadIdentity"), "useWorkloadIdentity and useInstancePrincipals cannot be used together")) } - if !c.UseInstancePrincipals && !c.UseWorkloadIdentity { + if !c.UseInstancePrincipals && !c.UseWorkloadIdentity && os.Getenv("OCI_CONFIG_FILE") == "" { allErrs = append(allErrs, validateAuthConfig(&c.Auth, field.NewPath("auth"))...) }