Merge pull request #129 from Vacant2333/chore-fix-problems

cleanup: fix problems

charts/karpenter/templates/poddisruptionbudget.yaml

@@ -10,7 +10,7 @@ metadata:
     {{- toYaml . | nindent 4 }}
   {{- end }}
 spec:
-  minAvailable: {{ .Values.podDisruptionBudget.maxUnavailable }}
+  minAvailable: {{ .Values.podDisruptionBudget.minAvailable }}
   selector:
     matchLabels:
     {{- include "karpenter.selectorLabels" . | nindent 6 }}

charts/karpenter/values.yaml

@@ -33,7 +33,7 @@ alibabacloud:
   region_id: ""
 
 podDisruptionBudget:
-  maxUnavailable: 1
+  minAvailable: 1
 
 controller:
   replicaCount: 2

pkg/cache/cache.go

@@ -27,7 +27,7 @@ const (
 	// DO NOT CHANGE THIS VALUE WITHOUT DUE CONSIDERATION
 	DefaultTTL = time.Minute
 	// KubernetesVersionTTL is the time before the detected Kubernetes version is removed from cache,
-	// to be re-detected next time it is needed.
+	// to be re-detected the next time it is needed.
 	KubernetesVersionTTL = 15 * time.Minute
 	// UnavailableOfferingsTTL is the time before offerings that were marked as unavailable
 	// are removed from the cache and are available for launch again

pkg/cloudprovider/drift.go

@@ -45,15 +45,15 @@ func (c *CloudProvider) isNodeClassDrifted(ctx context.Context, nodeClaim *karpv
 	if err != nil {
 		return "", err
 	}
-	securitygroupDrifted, err := c.areSecurityGroupsDrifted(instance, nodeClass)
+	securityGroupDrifted, err := c.areSecurityGroupsDrifted(instance, nodeClass)
 	if err != nil {
 		return "", fmt.Errorf("calculating securitygroup drift, %w", err)
 	}
 	vSwitchDrifted, err := c.isVSwitchDrifted(instance, nodeClass)
 	if err != nil {
 		return "", fmt.Errorf("calculating vSwitch drift, %w", err)
 	}
-	drifted := lo.FindOrElse([]cloudprovider.DriftReason{securitygroupDrifted, vSwitchDrifted}, "", func(i cloudprovider.DriftReason) bool {
+	drifted := lo.FindOrElse([]cloudprovider.DriftReason{securityGroupDrifted, vSwitchDrifted}, "", func(i cloudprovider.DriftReason) bool {
 		return string(i) != ""
 	})
 	return drifted, nil

pkg/cloudprovider/events/events.go

@@ -13,6 +13,7 @@ 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 events
 
 import (

pkg/controllers/controllers.go

@@ -53,12 +53,12 @@ func NewControllers(ctx context.Context, mgr manager.Manager, clk clock.Clock, r
 	cloudProvider cloudprovider.CloudProvider,
 	instanceProvider instance.Provider, instanceTypeProvider instancetype.Provider,
 	pricingProvider pricing.Provider,
-	vSwitchProvider vswitch.Provider, securitygroupProvider securitygroup.Provider,
+	vSwitchProvider vswitch.Provider, securityGroupProvider securitygroup.Provider,
 	imageProvider imagefamily.Provider) []controller.Controller {
 
 	controllers := []controller.Controller{
 		nodeclasshash.NewController(kubeClient),
-		nodeclaasstatus.NewController(kubeClient, vSwitchProvider, securitygroupProvider, imageProvider),
+		nodeclaasstatus.NewController(kubeClient, vSwitchProvider, securityGroupProvider, imageProvider),
 		nodeclasstermination.NewController(kubeClient, recorder),
 		controllerspricing.NewController(pricingProvider),
 		nodeclaimgarbagecollection.NewController(kubeClient, cloudProvider),

pkg/controllers/interruption/controller.go

@@ -92,9 +92,7 @@ func (c *Controller) Register(ctx context.Context, m manager.Manager) error {
 		Named("interruption").
 		For(&corev1.Node{}).
 		WithEventFilter(predicate.NewTypedPredicateFuncs(func(obj client.Object) bool {
-			lables := obj.GetLabels()
-
-			if lable, ok := lables[karpv1.CapacityTypeLabelKey]; !ok || lable != karpv1.CapacityTypeSpot {
+			if label, ok := obj.GetLabels()[karpv1.CapacityTypeLabelKey]; !ok || label != karpv1.CapacityTypeSpot {
 				return false
 			}
 

pkg/controllers/nodeclass/status/securitygroup.go

@@ -46,9 +46,11 @@ func (sg *SecurityGroup) Reconcile(ctx context.Context, nodeClass *v1alpha1.ECSN
 		// Returning 'ok' in this case means that the ecsnodeclass will remain in an unready state until the component is restarted.
 		return reconcile.Result{RequeueAfter: time.Second * 15}, nil
 	}
+
 	sort.Slice(securityGroups, func(i, j int) bool {
 		return *securityGroups[i].SecurityGroupId < *securityGroups[j].SecurityGroupId
 	})
+
 	nodeClass.Status.SecurityGroups = lo.Map(securityGroups, func(securityGroup *ecsclient.DescribeSecurityGroupsResponseBodySecurityGroupsSecurityGroup, _ int) v1alpha1.SecurityGroup {
 		return v1alpha1.SecurityGroup{
 			ID:   *securityGroup.SecurityGroupId,

pkg/operator/options/options_validation.go

@@ -22,13 +22,13 @@ import (
 	"go.uber.org/multierr"
 )
 
-func (o Options) Validate() error {
+func (o *Options) Validate() error {
 	return multierr.Combine(
 		o.validateRequiredFields(),
 	)
 }
 
-func (o Options) validateRequiredFields() error {
+func (o *Options) validateRequiredFields() error {
 	if o.ClusterID == "" {
 		return fmt.Errorf("missing field, cluster-id")
 	}

pkg/providers/instancetype/instancetype.go

@@ -369,28 +369,24 @@ func (p *DefaultProvider) createOfferings(_ context.Context, instanceType string
 
 		if odOK {
 			isUnavailable := p.unavailableOfferings.IsUnavailable(instanceType, zone.ID, karpv1.CapacityTypeOnDemand)
-			offeringAvailable := !isUnavailable && odOK && zone.Available
+			offeringAvailable := !isUnavailable && zone.Available
 
 			offerings = append(offerings, p.createOffering(zone.ID, karpv1.CapacityTypeOnDemand, odPrice, offeringAvailable))
 		}
 
 		if spotOK {
 			isUnavailable := p.unavailableOfferings.IsUnavailable(instanceType, zone.ID, karpv1.CapacityTypeSpot)
-			offeringAvailable := !isUnavailable && spotOK && zone.Available
+			offeringAvailable := !isUnavailable && zone.Available
 
 			offerings = append(offerings, p.createOffering(zone.ID, karpv1.CapacityTypeSpot, spotPrice, offeringAvailable))
 		}
 	}
 
-	if len(offerings) == 0 {
-		return nil
-	}
-
 	return offerings
 }
 
 func (p *DefaultProvider) createOffering(zone, capacityType string, price float64, available bool) cloudprovider.Offering {
-	offering := cloudprovider.Offering{
+	return cloudprovider.Offering{
 		Requirements: scheduling.NewRequirements(
 			scheduling.NewRequirement(karpv1.CapacityTypeLabelKey, corev1.NodeSelectorOpIn, capacityType),
 			scheduling.NewRequirement(corev1.LabelTopologyZone, corev1.NodeSelectorOpIn, zone),
@@ -399,8 +395,6 @@ func (p *DefaultProvider) createOffering(zone, capacityType string, price float6
 		Price:     price,
 		Available: available,
 	}
-
-	return offering
 }
 
 func (p *DefaultProvider) Reset() {

pkg/providers/instancetype/types.go

@@ -296,6 +296,7 @@ func pods(_ context.Context,
 	if lo.FromPtr(podsPerCore) > 0 {
 		count = lo.Min([]int64{int64(lo.FromPtr(podsPerCore) * lo.FromPtr(info.CpuCoreCount)), count})
 	}
+
 	return resources.Quantity(fmt.Sprint(count))
 }
 
@@ -349,19 +350,6 @@ func privateIPv4Address(info *ecsclient.DescribeInstanceTypesResponseBodyInstanc
 	return resources.Quantity(fmt.Sprint(*info.EniPrivateIpAddressQuantity * (*info.EniQuantity)))
 }
 
-func getInstanceBandwidth(info *ecsclient.DescribeInstanceTypesResponseBodyInstanceTypesInstanceType) int64 {
-	bandwidthRx := int32(0)
-	bandwidthTx := int32(0)
-	if info.InstanceBandwidthRx != nil {
-		bandwidthRx = *info.InstanceBandwidthRx
-	}
-	if info.InstanceBandwidthTx != nil {
-		bandwidthTx = *info.InstanceBandwidthTx
-	}
-
-	if bandwidthRx > bandwidthTx {
-		return int64(bandwidthRx)
-	}
-
-	return int64(bandwidthTx)
+func getInstanceBandwidth(info *ecsclient.DescribeInstanceTypesResponseBodyInstanceTypesInstanceType) int32 {
+	return max(lo.FromPtr(info.InstanceBandwidthRx), lo.FromPtr(info.InstanceBandwidthTx))
 }

pkg/providers/vswitch/vswitch.go

@@ -121,7 +121,8 @@ func (p *DefaultProvider) List(ctx context.Context, nodeClass *v1alpha1.ECSNodeC
 			// we are accepting it for now, as this will be an insignificant amount of memory
 			p.availableIPAddressCache.SetDefault(lo.FromPtr(vSwitch.VSwitchId), lo.FromPtr(vSwitch.AvailableIpAddressCount))
 
-			delete(p.inflightIPs, lo.FromPtr(vSwitch.VSwitchId)) // remove any previously tracked IP addresses since we just refreshed from ECS
+			// remove any previously tracked IP addresses since we just refreshed from ECS
+			delete(p.inflightIPs, lo.FromPtr(vSwitch.VSwitchId))
 		}); err != nil {
 			return nil, fmt.Errorf("describing vSwitches %s, %w", pretty.Concise(selectorTerms), err)
 		}