From 814044ff07c1d0a881fe58f828230565190ac17b Mon Sep 17 00:00:00 2001 From: Louis PORTAY Date: Mon, 1 Aug 2022 14:31:27 +0200 Subject: [PATCH] fix: added comments for linter --- .../scaleway/scaleway_cloud_provider.go | 1 + .../scaleway/scaleway_node_group.go | 42 ++++++++--- .../scalewaygo/scaleway_kapsule_api.go | 69 +++++++++++++++---- 3 files changed, 90 insertions(+), 22 deletions(-) diff --git a/cluster-autoscaler/cloudprovider/scaleway/scaleway_cloud_provider.go b/cluster-autoscaler/cloudprovider/scaleway/scaleway_cloud_provider.go index 10919f9b14f9..949fe26c9bfb 100644 --- a/cluster-autoscaler/cloudprovider/scaleway/scaleway_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/scaleway/scaleway_cloud_provider.go @@ -100,6 +100,7 @@ func newScalewayCloudProvider(configFile io.Reader, defaultUserAgent string, rl } } +// BuildScaleway returns CloudProvider implementation for Scaleway. func BuildScaleway( opts config.AutoscalingOptions, do cloudprovider.NodeGroupDiscoveryOptions, diff --git a/cluster-autoscaler/cloudprovider/scaleway/scaleway_node_group.go b/cluster-autoscaler/cloudprovider/scaleway/scaleway_node_group.go index de9b9e5e1604..5704e4f313e6 100644 --- a/cluster-autoscaler/cloudprovider/scaleway/scaleway_node_group.go +++ b/cluster-autoscaler/cloudprovider/scaleway/scaleway_node_group.go @@ -32,10 +32,8 @@ import ( "strings" ) -var ( - ErrNodeNotInPool = errors.New("node marked for deletion and not found in pool") -) - +// NodeGroup implements cloudprovider.NodeGroup interface. +// it is used to resize a Scaleway Pool which is a group of nodes with the same capacity. type NodeGroup struct { scalewaygo.Client @@ -44,23 +42,32 @@ type NodeGroup struct { p *scalewaygo.Pool } +// MaxSize returns maximum size of the node group. func (ng *NodeGroup) MaxSize() int { klog.V(6).Info("MaxSize,called") return int(ng.p.MaxSize) } +// MinSize returns minimum size of the node group. func (ng *NodeGroup) MinSize() int { klog.V(6).Info("MinSize,called") return int(ng.p.MinSize) } +// TargetSize returns the current target size of the node group. It is possible that the +// number of nodes in Kubernetes is different at the moment but should be equal +// to Size() once everything stabilizes (new nodes finish startup and registration or +// removed nodes are deleted completely). func (ng *NodeGroup) TargetSize() (int, error) { klog.V(6).Info("TargetSize,called") return int(ng.p.Size), nil } +// IncreaseSize increases the size of the node group. To delete a node you need +// to explicitly name it and use DeleteNode. This function should wait until +// node group size is updated. func (ng *NodeGroup) IncreaseSize(delta int) error { klog.V(4).Infof("IncreaseSize,ClusterID=%s,delta=%d", ng.p.ClusterID, delta) @@ -94,6 +101,9 @@ func (ng *NodeGroup) IncreaseSize(delta int) error { return nil } +// DeleteNodes deletes nodes from this node group. Error is returned either on +// failure or if the given node doesn't belong to this node group. This function +// should wait until node group size is updated. func (ng *NodeGroup) DeleteNodes(nodes []*apiv1.Node) error { ctx := context.Background() klog.V(4).Info("DeleteNodes,", len(nodes), " nodes to reclaim") @@ -101,7 +111,7 @@ func (ng *NodeGroup) DeleteNodes(nodes []*apiv1.Node) error { node, ok := ng.nodes[n.Spec.ProviderID] if !ok { - klog.Errorf("DeleteNodes,ProviderID=%s,PoolID=%s,%s", n.Spec.ProviderID, ng.p.ID, ErrNodeNotInPool) + klog.Errorf("DeleteNodes,ProviderID=%s,PoolID=%s,node marked for deletion not found in pool", n.Spec.ProviderID, ng.p.ID) continue } @@ -119,6 +129,11 @@ func (ng *NodeGroup) DeleteNodes(nodes []*apiv1.Node) error { return nil } +// DecreaseTargetSize decreases the target size of the node group. This function +// doesn't permit to delete any existing node and can be used only to reduce the +// request for new nodes that have not been yet fulfilled. Delta should be negative. +// It is assumed that cloud provider will not delete the existing nodes when there +// is an option to just decrease the target. func (ng *NodeGroup) DecreaseTargetSize(delta int) error { klog.V(4).Infof("DecreaseTargetSize,ClusterID=%s,delta=%d", ng.p.ClusterID, delta) @@ -151,15 +166,18 @@ func (ng *NodeGroup) DecreaseTargetSize(delta int) error { return nil } +// Id returns an unique identifier of the node group. func (ng *NodeGroup) Id() string { return ng.p.ID } +// Debug returns a string containing all information regarding this node group. func (ng *NodeGroup) Debug() string { klog.V(4).Info("Debug,called") return fmt.Sprintf("id:%s,status:%s,version:%s,autoscaling:%t,size:%d,min_size:%d,max_size:%d", ng.Id(), ng.p.Status, ng.p.Version, ng.p.Autoscaling, ng.p.Size, ng.MinSize(), ng.MaxSize()) } +// Nodes returns a list of all nodes that belong to this node group. func (ng *NodeGroup) Nodes() ([]cloudprovider.Instance, error) { var nodes []cloudprovider.Instance @@ -218,7 +236,7 @@ func (ng *NodeGroup) TemplateNodeInfo() (*schedulerframework.NodeInfo, error) { } func parseTaints(taints map[string]string) []apiv1.Taint { - k8s_taints := make([]apiv1.Taint, 0, len(taints)) + k8sTaints := make([]apiv1.Taint, 0, len(taints)) for key, valueEffect := range taints { @@ -240,11 +258,13 @@ func parseTaints(taints map[string]string) []apiv1.Taint { } taint.Key = key - k8s_taints = append(k8s_taints, taint) + k8sTaints = append(k8sTaints, taint) } - return k8s_taints + return k8sTaints } +// Exist checks if the node group really exists on the cloud provider side. Allows to tell the +// theoretical node group from the real one. func (ng *NodeGroup) Exist() bool { klog.V(4).Infof("Exist,PoolID=%s", ng.p.ID) @@ -261,14 +281,17 @@ func (ng *NodeGroup) Exist() bool { // Pool Autoprovision feature is not supported by Scaleway +// Create creates the node group on the cloud provider side. func (ng *NodeGroup) Create() (cloudprovider.NodeGroup, error) { return nil, cloudprovider.ErrNotImplemented } +// Delete deletes the node group on the cloud provider side. func (ng *NodeGroup) Delete() error { return cloudprovider.ErrNotImplemented } +// Autoprovisioned returns true if the node group is autoprovisioned. func (ng *NodeGroup) Autoprovisioned() bool { return false } @@ -302,7 +325,8 @@ func fromScwStatus(status scalewaygo.NodeStatus) *cloudprovider.InstanceStatus { switch status { case scalewaygo.NodeStatusReady: st.State = cloudprovider.InstanceRunning - case scalewaygo.NodeStatusCreating, scalewaygo.NodeStatusNotReady, + case scalewaygo.NodeStatusCreating, scalewaygo.NodeStatusStarting, + scalewaygo.NodeStatusRegistering, scalewaygo.NodeStatusNotReady, scalewaygo.NodeStatusUpgrading, scalewaygo.NodeStatusRebooting: st.State = cloudprovider.InstanceCreating case scalewaygo.NodeStatusDeleting: diff --git a/cluster-autoscaler/cloudprovider/scaleway/scalewaygo/scaleway_kapsule_api.go b/cluster-autoscaler/cloudprovider/scaleway/scalewaygo/scaleway_kapsule_api.go index 35db32f2bda2..be6677de8a31 100644 --- a/cluster-autoscaler/cloudprovider/scaleway/scalewaygo/scaleway_kapsule_api.go +++ b/cluster-autoscaler/cloudprovider/scaleway/scalewaygo/scaleway_kapsule_api.go @@ -39,15 +39,25 @@ const ( ) var ( + // ErrMissingClusterID is returned when no cluster id has been found + // either in env variables or in config file ErrMissingClusterID = errors.New("cluster ID is not provided") + // ErrMissingSecretKey is returned when no secret key has been found + // either in env variables or in config file ErrMissingSecretKey = errors.New("scaleway secret key is not provided") - ErrMissingRegion = errors.New("region is not provided") + // ErrMissingRegion is returned when no region has been found + // either in env variables or in config file + ErrMissingRegion = errors.New("region is not provided") + // ErrClientSide indicates an error on user side ErrClientSide = errors.New("400 error type") + // ErrServerSide indicates an error on server side ErrServerSide = errors.New("500 error type") - ErrOther = errors.New("generic error type") + // ErrOther indicates a generic HTTP error + ErrOther = errors.New("generic error type") ) +// Config is used to deserialize config file passed with flag `cloud-config` type Config struct { ClusterID string `json:"cluster_id"` SecretKey string `json:"secret_key"` @@ -100,6 +110,7 @@ type scalewayRequest struct { // Listing queries default to `fetch all resources` if no `page` is provided // as CA needs access to all nodes and pools +// Client is used to talk to Scaleway Kapsule API type Client interface { GetPool(ctx context.Context, req *GetPoolRequest) (*Pool, error) ListPools(ctx context.Context, req *ListPoolsRequest) (*ListPoolsResponse, error) @@ -108,7 +119,7 @@ type Client interface { DeleteNode(ctx context.Context, req *DeleteNodeRequest) (*Node, error) } -// client is the struct to perform API calls +// client contains necessary information to perform API calls type client struct { httpClient *http.Client apiURL string @@ -202,21 +213,35 @@ func (c *client) do(ctx context.Context, req *scalewayRequest, res interface{}) return fmt.Errorf("%d %v %v: %w", httpResponse.StatusCode, httpRequest.Method, httpRequest.URL, err) } +// NodeStatus is the state in which a node might be type NodeStatus string const ( - NodeStatusUnknown = NodeStatus("unknown") - NodeStatusCreating = NodeStatus("creating") - NodeStatusNotReady = NodeStatus("not_ready") - NodeStatusReady = NodeStatus("ready") - NodeStatusDeleting = NodeStatus("deleting") - NodeStatusDeleted = NodeStatus("deleted") - NodeStatusLocked = NodeStatus("locked") - NodeStatusRebooting = NodeStatus("rebooting") + // NodeStatusCreating indicates that node is provisioning the underlying instance/BM + NodeStatusCreating = NodeStatus("creating") + // NodeStatusStarting indicates that node is being configured and/or booting + NodeStatusStarting = NodeStatus("starting") + // NodeStatusRegistering indicates that underlying node has booted and k8s services are starting + NodeStatusRegistering = NodeStatus("registering") + // NodeStatusNotReady indicates that k8s has marked this node as `NotReady` + NodeStatusNotReady = NodeStatus("not_ready") + // NodeStatusReady indicates that node is ready for use + NodeStatusReady = NodeStatus("ready") + // NodeStatusDeleting indicates that node is being deleted + NodeStatusDeleting = NodeStatus("deleting") + // NodeStatusDeleted indicates that node is deleted + NodeStatusDeleted = NodeStatus("deleted") + // NodeStatusLocked indicates that node has been locked for legal reasons + NodeStatusLocked = NodeStatus("locked") + // NodeStatusRebooting indicates that node is rebooting + NodeStatusRebooting = NodeStatus("rebooting") + // NodeStatusCreationError indicates that node failed to create NodeStatusCreationError = NodeStatus("creation_error") - NodeStatusUpgrading = NodeStatus("upgrading") + // NodeStatusUpgrading indicates that this node CP is currently upgrading k8s version + NodeStatusUpgrading = NodeStatus("upgrading") ) +// Node represents an instance running in a scaleway pool type Node struct { // ID: the ID of the node ID string `json:"id"` @@ -236,10 +261,11 @@ type Node struct { UpdatedAt *time.Time `json:"updated_at"` } +// PoolStatus is the state in which a pool might be (unused) type PoolStatus string +// These are possible statuses for a scaleway pool const ( - PoolStatusUnknown = PoolStatus("unknown") PoolStatusReady = PoolStatus("ready") PoolStatusDeleting = PoolStatus("deleting") PoolStatusDeleted = PoolStatus("deleted") @@ -249,6 +275,7 @@ const ( PoolStatusUpgrading = PoolStatus("upgrading") ) +// Pool is the abstraction used to gather nodes with the same specs type Pool struct { // ID: the ID of the pool ID string `json:"id"` @@ -278,11 +305,13 @@ type Pool struct { Zone string `json:"zone"` } +// GetPoolRequest is passed to `GetPool` method type GetPoolRequest struct { // PoolID: the ID of the requested pool PoolID string `json:"-"` } +// GetPool is used to request a Pool by its id func (c *client) GetPool(ctx context.Context, req *GetPoolRequest) (*Pool, error) { var err error @@ -306,6 +335,8 @@ func (c *client) GetPool(ctx context.Context, req *GetPoolRequest) (*Pool, error return &resp, nil } +// ListPoolsRequest is passed to `ListPools` method +// it can be used for optional pagination type ListPoolsRequest struct { // the ID of the cluster from which the pools will be listed from ClusterID string `json:"-"` @@ -315,6 +346,8 @@ type ListPoolsRequest struct { PageSize *uint32 `json:"-"` } +// GenericNodeSpecs represents NodeType specs used for scale-up simulations. +// it is used to select the appropriate pool to scale-up. type GenericNodeSpecs struct { NodePricePerHour float32 `json:"node_price_per_hour"` MaxPods uint32 `json:"max_pods"` @@ -329,11 +362,13 @@ type GenericNodeSpecs struct { Taints map[string]string `json:"taints"` } +// PoolWithGenericNodeSpecs contains the requested `Pool` with additional `Specs` information type PoolWithGenericNodeSpecs struct { Pool *Pool `json:"pool"` Specs GenericNodeSpecs `json:"specs"` } +// ListPoolsResponse is returned from `ListPools` method type ListPoolsResponse struct { // TotalCount: the total number of pools that exists for the cluster TotalCount uint32 `json:"total_count"` @@ -341,6 +376,7 @@ type ListPoolsResponse struct { Pools []*PoolWithGenericNodeSpecs `json:"pools"` } +// ListPools returns pools associated to a cluster id, pagination optional func (c *client) ListPools(ctx context.Context, req *ListPoolsRequest) (*ListPoolsResponse, error) { klog.V(4).Info("ListPools,ClusterID=", req.ClusterID) @@ -418,6 +454,7 @@ func (c *client) listPoolsPaginated(ctx context.Context, req *ListPoolsRequest) return &resp, nil } +// UpdatePoolRequest is passed to `UpdatePool` method type UpdatePoolRequest struct { // PoolID: the ID of the pool to update PoolID string `json:"-"` @@ -425,6 +462,7 @@ type UpdatePoolRequest struct { Size *uint32 `json:"size"` } +// UpdatePool is used to resize a pool, to decrease pool size `DeleteNode` should be used instead func (c *client) UpdatePool(ctx context.Context, req *UpdatePoolRequest) (*Pool, error) { var err error @@ -454,6 +492,7 @@ func (c *client) UpdatePool(ctx context.Context, req *UpdatePoolRequest) (*Pool, return &resp, nil } +// ListNodesRequest is passed to `ListNodes` method type ListNodesRequest struct { // ClusterID: the cluster ID from which the nodes will be listed from ClusterID string `json:"-"` @@ -465,6 +504,7 @@ type ListNodesRequest struct { PageSize *uint32 `json:"-"` } +// ListNodesResponse is returned from `ListNodes` method type ListNodesResponse struct { // TotalCount: the total number of nodes TotalCount uint32 `json:"total_count"` @@ -472,6 +512,7 @@ type ListNodesResponse struct { Nodes []*Node `json:"nodes"` } +// ListNodes returns the Nodes associated to a Cluster and/or a Pool func (c *client) ListNodes(ctx context.Context, req *ListNodesRequest) (*ListNodesResponse, error) { klog.V(4).Info("ListNodes,ClusterID=", req.ClusterID) @@ -553,10 +594,12 @@ func (c *client) listNodesPaginated(ctx context.Context, req *ListNodesRequest) return &resp, nil } +// DeleteNodeRequest is passed to `DeleteNode` method type DeleteNodeRequest struct { NodeID string `json:"-"` } +// DeleteNode asynchronously deletes a Node by its id func (c *client) DeleteNode(ctx context.Context, req *DeleteNodeRequest) (*Node, error) { var err error