Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: use new property name for node count property + specify the core properties (node count, total/allocatable/available CPU/memory) #810

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion apis/cluster/v1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion apis/cluster/v1beta1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion apis/placement/v1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion apis/placement/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion apis/placement/v1beta1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion apis/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 5 additions & 5 deletions cmd/memberagent/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,15 @@ import (
workv1alpha1controller "go.goms.io/fleet/pkg/controllers/workv1alpha1"
fleetmetrics "go.goms.io/fleet/pkg/metrics"
"go.goms.io/fleet/pkg/propertyprovider"
"go.goms.io/fleet/pkg/propertyprovider/aks"
"go.goms.io/fleet/pkg/propertyprovider/azure"
"go.goms.io/fleet/pkg/utils"
"go.goms.io/fleet/pkg/utils/httpclient"
//+kubebuilder:scaffold:imports
)

const (
// The list of available property provider names.
aksPropertyProvider = "azure"
azurePropertyProvider = "azure"
)

var (
Expand Down Expand Up @@ -351,11 +351,11 @@ func Start(ctx context.Context, hubCfg, memberConfig *rest.Config, hubOpts, memb
// Set up a provider provider (if applicable).
var pp propertyprovider.PropertyProvider
switch {
case propertyProvider != nil && *propertyProvider == aksPropertyProvider:
klog.V(2).Info("setting up the AKS property provider")
case propertyProvider != nil && *propertyProvider == azurePropertyProvider:
klog.V(2).Info("setting up the Azure property provider")
// Note that the property provider, though initialized here, is not started until
// the specific instance wins the leader election.
pp = aks.New(region)
pp = azure.New(region)
default:
// Fall back to not using any property provider if the provided type is none or
// not recognizable.
Expand Down
19 changes: 15 additions & 4 deletions docs/concepts/PropertyProviderAndClusterProperties/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ in the form of Kubernetes conditions.

The Fleet member agent can run with or without a property provider. If a provider is not set up, or
the given provider fails to start properly, the agent will collect limited properties about
the cluster on its own, specifically the total and allocatable CPU and memory capacities of
the host member cluster.
the cluster on its own, specifically the node count, plus the total/allocatable
CPU and memory capacities of the host member cluster.

## Cluster properties

Expand All @@ -75,7 +75,7 @@ such as `cpu` and `memory`, and the usage information should consist of:
* Non-resource property: a metric about a member cluster, in the form of a key/value
pair; the key should be in the format of
[a Kubernetes label key](https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#syntax-and-character-set),
such as `kubernetes.azure.com/node-count`, and the value at this moment should be a sortable
such as `kubernetes-fleet.io/node-count`, and the value at this moment should be a sortable
numeric that can be parsed as
[a Kubernetes quantity](https://kubernetes.io/docs/reference/kubernetes-api/common-definitions/quantity/).

Expand All @@ -92,7 +92,7 @@ status:
agentStatus: ...
conditions: ...
properties:
kubernetes.azure.com/node-count:
kubernetes-fleet.io/node-count:
observationTime: "2024-04-30T14:54:24Z"
value: "2"
...
Expand All @@ -111,3 +111,14 @@ status:
Note that conditions reported by the property provider (if any), would be available in the
`.status.conditions` array as well.

### Core properties

The following properties are considered core properties in Fleet, which should be supported
in all property provider implementations. Fleet agents will collect them even when no
property provider has been set up.

| Property Type | Name | Description |
| ------------- | ---- | ----------- |
| Non-resource property | `kubernetes-fleet.io/node-count` | The number of nodes in a cluster. |
| Resource property | `cpu` | The usage information (total, allocatable, and available capacity) of CPU resource in a cluster. |
| Resource property | `memory` | The usage information (total, allocatable, and available capacity) of memory resource in a cluster. |
6 changes: 3 additions & 3 deletions docs/howtos/property-based-scheduling.md
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ spec:
clusterSelectorTerms:
- propertySelector:
matchExpressions:
- name: "kubernetes.azure.com/node-count"
- name: "kubernetes-fleet.io/node-count"
operator: Ge
values:
- "5"
Expand Down Expand Up @@ -130,7 +130,7 @@ spec:
region: east
propertySelector:
matchExpressions:
- name: "kubernetes.azure.com/node-count"
- name: "kubernetes-fleet.io/node-count"
operator: Ge
values:
- "5"
Expand Down Expand Up @@ -225,7 +225,7 @@ spec:
- weight: 20
preference:
metricSorter:
name: kubernetes.azure.com/node-count
name: kubernetes-fleet.io/node-count
sortOrder: Descending
```

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,12 @@ func (r *Reconciler) updateResourceStats(ctx context.Context, imc *clusterv1beta
allocatableMemory.Add(*(node.Status.Allocatable.Memory()))
}

imc.Status.Properties = map[clusterv1beta1.PropertyName]clusterv1beta1.PropertyValue{
propertyprovider.NodeCountProperty: {
Value: fmt.Sprintf("%d", len(nodes.Items)),
ObservationTime: metav1.Now(),
},
}
imc.Status.ResourceUsage.Capacity = corev1.ResourceList{
corev1.ResourceCPU: capacityCPU,
corev1.ResourceMemory: capacityMemory,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (

clusterv1beta1 "go.goms.io/fleet/apis/cluster/v1beta1"
"go.goms.io/fleet/pkg/controllers/work"
"go.goms.io/fleet/pkg/propertyprovider"
"go.goms.io/fleet/pkg/utils"
)

Expand Down Expand Up @@ -136,6 +137,7 @@ var _ = Describe("Test Internal Member Cluster Controller", Serial, func() {
Expect(updatedHealthCond.Reason).To(Equal(EventReasonInternalMemberClusterHealthy))

By("checking updated member cluster usage")
Expect(imc.Status.Properties[propertyprovider.NodeCountProperty].Value).ShouldNot(BeEmpty())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we know how many nodes does this cluster have?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the dummy env I am not sure, but there will be a PR that re-factors the tests for this controller and we will using the full comparison like the one in the E2E tests for this part.

Expect(imc.Status.ResourceUsage.Allocatable).ShouldNot(BeNil())
Expect(imc.Status.ResourceUsage.Capacity).ShouldNot(BeNil())
Expect(imc.Status.ResourceUsage.ObservationTime).ToNot(Equal(metav1.Now()))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ Licensed under the MIT license.
*/

// Package controllers feature a number of controllers that are in use
// by the AKS property provider.
// by the Azure property provider.
package controllers

import (
Expand All @@ -17,7 +17,7 @@ import (
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"

"go.goms.io/fleet/pkg/propertyprovider/aks/trackers"
"go.goms.io/fleet/pkg/propertyprovider/azure/trackers"
)

// NodeReconciler reconciles Node objects.
Expand All @@ -30,10 +30,10 @@ type NodeReconciler struct {
func (r *NodeReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
nodeRef := klog.KRef(req.Namespace, req.Name)
startTime := time.Now()
klog.V(2).InfoS("Reconciliation starts for node objects in the AKS property provider", "node", nodeRef)
klog.V(2).InfoS("Reconciliation starts for node objects in the Azure property provider", "node", nodeRef)
defer func() {
latency := time.Since(startTime).Milliseconds()
klog.V(2).InfoS("Reconciliation ends for node objects in the AKS property provider", "node", nodeRef, "latency", latency)
klog.V(2).InfoS("Reconciliation ends for node objects in the Azure property provider", "node", nodeRef, "latency", latency)
}()

// Retrieve the node object.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ Licensed under the MIT license.
*/

// Package controllers feature a number of controllers that are in use
// by the AKS property provider.
// by the Azure property provider.
package controllers

import (
Expand All @@ -17,7 +17,7 @@ import (
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"

"go.goms.io/fleet/pkg/propertyprovider/aks/trackers"
"go.goms.io/fleet/pkg/propertyprovider/azure/trackers"
)

// TO-DO (chenyu1): this is a relatively expensive watcher, due to how frequent pods can change
Expand All @@ -35,10 +35,10 @@ type PodReconciler struct {
func (p *PodReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
podRef := klog.KRef(req.Namespace, req.Name)
startTime := time.Now()
klog.V(2).InfoS("Reconciliation starts for pod objects in the AKS property provider", "pod", podRef)
klog.V(2).InfoS("Reconciliation starts for pod objects in the Azure property provider", "pod", podRef)
defer func() {
latency := time.Since(startTime).Milliseconds()
klog.V(2).InfoS("Reconciliation ends for pod objects in the AKS property provider", "pod", podRef, "latency", latency)
klog.V(2).InfoS("Reconciliation ends for pod objects in the Azure property provider", "pod", podRef, "latency", latency)
}()

// Retrieve the pod object.
Expand Down
Loading
Loading