Skip to content
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
4d00f25
add priority to CNRs
dnorman3 Aug 11, 2025
18e4f6f
ensure backward compatibility
dnorman3 Aug 11, 2025
da244fa
rm priority
dnorman3 Aug 11, 2025
f01af16
moved filter logic to function
dnorman3 Aug 11, 2025
8c3e152
rm priority
dnorman3 Aug 11, 2025
797c689
added metrics
dnorman3 Aug 11, 2025
c491336
use max()
dnorman3 Aug 11, 2025
d58dce4
rename function
dnorman3 Aug 12, 2025
ed50518
use max()
dnorman3 Aug 12, 2025
9a1baea
add tests
dnorman3 Aug 12, 2025
ec68f28
rename to batchPriority for clarity
dnorman3 Aug 12, 2025
78f3273
make names clearer
dnorman3 Aug 12, 2025
f7ec290
update metrics name
dnorman3 Aug 12, 2025
70f3323
rm Priority
dnorman3 Aug 12, 2025
9ab7a5a
make names clearer
dnorman3 Aug 12, 2025
65cdbf9
added comments
dnorman3 Aug 12, 2025
2be3d22
fix tab
dnorman3 Aug 12, 2025
4a5d699
fix tab
dnorman3 Aug 12, 2025
63e856b
add complex test case
dnorman3 Aug 12, 2025
0839bfc
met metrics to counter and update names
dnorman3 Aug 12, 2025
4468e29
set metrics to counter
dnorman3 Aug 12, 2025
34abc20
allow negative values
dnorman3 Aug 12, 2025
b187dbd
add priority
dnorman3 Aug 12, 2025
a95dfb1
Merge branch 'master' into dnorman3/priority-system
dnorman3 Aug 12, 2025
25e602a
bumped Go version
dnorman3 Aug 12, 2025
de0b25f
Replace local getPriority func with direct field access
dnorman3 Aug 13, 2025
a34bcec
reference negavtive numbers
dnorman3 Aug 13, 2025
9104133
add value range
dnorman3 Aug 13, 2025
73ad501
rm "len(lowestPriorityBatch)" as selectLowestPriorityNodeGroups alway…
dnorman3 Aug 13, 2025
b25458a
bump k8s version
dnorman3 Aug 13, 2025
116933f
added logging
dnorman3 Aug 14, 2025
0a5ccc9
remove node groups with concurrency set to 0
dnorman3 Aug 18, 2025
a77c634
rm metrics
dnorman3 Aug 18, 2025
c8e791b
add tests
dnorman3 Aug 20, 2025
bb6e575
add NodeGroupInfo metric
dnorman3 Aug 20, 2025
091a7aa
add NodeGroupChangeStatus
dnorman3 Aug 20, 2025
1621d15
observeChanges now returns a map
dnorman3 Aug 20, 2025
10148d5
observeChages now returns map and added metric
dnorman3 Aug 20, 2025
c8730da
metrics before return statement
dnorman3 Aug 21, 2025
38bda70
use nodegroup_name
dnorman3 Aug 21, 2025
89fab8a
rm redundant statement
dnorman3 Aug 25, 2025
75e7651
version 1.10.2
dnorman3 Aug 25, 2025
457dcd1
rm redundant test
dnorman3 Aug 25, 2025
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
4 changes: 4 additions & 0 deletions pkg/apis/atlassian/v1/nodegroup_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ type NodeGroupSpec struct {

// SkipPreTerminationChecks is an optional flag to skip pre-termination checks during cycling
SkipPreTerminationChecks bool `json:"skipPreTerminationChecks,omitempty"`

// Priority controls the ordering of CNR creation for this NodeGroup.
// Lower values are higher priority. 0 runs first, then 1, 2, ...
Comment thread
dnorman3 marked this conversation as resolved.
Outdated
Priority int32 `json:"priority,omitempty"`
}

// NodeGroupStatus defines the observed state of NodeGroup
Expand Down
7 changes: 5 additions & 2 deletions pkg/generation/cnr.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,10 @@ func GenerateCNR(nodeGroup atlassianv1.NodeGroup, nodes []string, name, namespac
}
}

return atlassianv1.CycleNodeRequest{
// Default/normalize priority for backward compatibility (not stored on CNR anymore)
_ = max(nodeGroup.Spec.Priority, 0)
Comment thread
dnorman3 marked this conversation as resolved.
Outdated

return atlassianv1.CycleNodeRequest{
Comment thread
dnorman3 marked this conversation as resolved.
Outdated
ObjectMeta: metav1.ObjectMeta{
Name: finalName,
Namespace: namespace,
Expand All @@ -111,7 +114,7 @@ func GenerateCNR(nodeGroup atlassianv1.NodeGroup, nodes []string, name, namespac
HealthChecks: nodeGroup.Spec.HealthChecks,
PreTerminationChecks: nodeGroup.Spec.PreTerminationChecks,
SkipInitialHealthChecks: nodeGroup.Spec.SkipInitialHealthChecks,
SkipPreTerminationChecks: nodeGroup.Spec.SkipPreTerminationChecks,
SkipPreTerminationChecks: nodeGroup.Spec.SkipPreTerminationChecks,
ValidationOptions: nodeGroup.Spec.ValidationOptions,
},
}
Expand Down
122 changes: 94 additions & 28 deletions pkg/observer/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -273,31 +273,77 @@ func (c *controller) dropInProgressNodeGroups(nodeGroups v1.NodeGroupList, cnrs

// createCNRs generates and applies CNRs from the changedNodeGroups
func (c *controller) createCNRs(changedNodeGroups []*ListedNodeGroups) {
Comment thread
dnorman3 marked this conversation as resolved.
klog.V(3).Infoln("applying")
for _, nodeGroup := range changedNodeGroups {
nodeNames := make([]string, 0, len(nodeGroup.List))
for _, node := range nodeGroup.List {
nodeNames = append(nodeNames, node.Name)
}
// generate cnr with prefix and use generate name method
cnr := generation.GenerateCNR(*nodeGroup.NodeGroup, nodeNames, c.CNRPrefix, c.Namespace)
generation.UseGenerateNameCNR(&cnr)
generation.GiveReason(&cnr, nodeGroup.Reason)
generation.SetAPIVersion(&cnr, apiVersion)
klog.V(3).Infoln("applying")
for _, nodeGroup := range changedNodeGroups {
nodeNames := make([]string, 0, len(nodeGroup.List))
for _, node := range nodeGroup.List {
nodeNames = append(nodeNames, node.Name)
}
// generate cnr with prefix and use generate name method
cnr := generation.GenerateCNR(*nodeGroup.NodeGroup, nodeNames, c.CNRPrefix, c.Namespace)
generation.UseGenerateNameCNR(&cnr)
generation.GiveReason(&cnr, nodeGroup.Reason)
generation.SetAPIVersion(&cnr, apiVersion)

name := generation.GetName(cnr.ObjectMeta)

if err := generation.ApplyCNR(c.client, c.DryMode, cnr); err != nil {
klog.Errorf("failed to apply cnr %q for nodegroup %q: %s", name, nodeGroup.NodeGroup.Name, err)
} else {
var drymodeStr string
if c.DryMode {
drymodeStr = "[drymode] "
}
klog.V(2).Infof("%ssuccessfully applied cnr %q for nodegroup %q", drymodeStr, name, nodeGroup.NodeGroup.Name)
c.CNRsCreated.WithLabelValues(nodeGroup.NodeGroup.Name).Inc()
}
}
}

name := generation.GetName(cnr.ObjectMeta)
// filterNodeGroupsByPriority returns only the node groups at the lowest priority value
// Backward compatibility: negative or missing priorities are treated as 0
func (c *controller) filterNodeGroupsByPriority(changedNodeGroups []*ListedNodeGroups) []*ListedNodeGroups {
if len(changedNodeGroups) == 0 {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this is a redundant check because changedNodeGroups will never be 0 here since you already check there are some prior to running this function, and if nil were ever to be returned here anyway that would currently crash observer because we check the first element of a nil list right after the call of this function.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yes please create and return an empty list if you end up keeping this check.

return nil
}
getPriority := func(ng *v1.NodeGroup) int32 { return max(ng.Spec.Priority, 0) }
Comment thread
dnorman3 marked this conversation as resolved.
Outdated
minPriority := getPriority(changedNodeGroups[0].NodeGroup)
for i := 1; i < len(changedNodeGroups); i++ {
Comment thread
dnorman3 marked this conversation as resolved.
p := getPriority(changedNodeGroups[i].NodeGroup)
if p < minPriority {
minPriority = p
}
}
var filtered []*ListedNodeGroups
for _, g := range changedNodeGroups {
if getPriority(g.NodeGroup) == minPriority {
filtered = append(filtered, g)
}
}
return filtered
Comment thread
dnorman3 marked this conversation as resolved.
}
Comment thread
dnorman3 marked this conversation as resolved.

if err := generation.ApplyCNR(c.client, c.DryMode, cnr); err != nil {
klog.Errorf("failed to apply cnr %q for nodegroup %q: %s", name, nodeGroup.NodeGroup.Name, err)
} else {
var drymodeStr string
if c.DryMode {
drymodeStr = "[drymode] "
}
klog.V(2).Infof("%ssuccessfully applied cnr %q for nodegroup %q", drymodeStr, name, nodeGroup.NodeGroup.Name)
c.CNRsCreated.WithLabelValues(nodeGroup.NodeGroup.Name).Inc()
}
}
// isLowerPriorityInProgress returns true if any in-progress CNR belongs to a NodeGroup with
// a priority lower than the provided minPriority (i.e., must finish before creating higher priorities)
func (c *controller) isLowerPriorityInProgress(minPriority int32, inProg v1.CycleNodeRequestList) bool {
if len(inProg.Items) == 0 {
return false
}
// Build a list of valid nodegroups to map CNRs to priorities
allNG := c.validNodeGroups()
Comment thread
dnorman3 marked this conversation as resolved.
Outdated
for _, cnr := range inProg.Items {
for i := range allNG.Items {
Comment thread
dnorman3 marked this conversation as resolved.
Outdated
ng := allNG.Items[i]
if cnr.IsFromNodeGroup(ng) {
p := max(ng.Spec.Priority, 0)
if p < minPriority {
return true
}
break
}
}
}
return false
}

// nextRunTime returns the next time the controller loop will run from now in UTC
Expand All @@ -319,8 +365,9 @@ func (c *controller) Run() {
nodeGroups = c.dropInProgressNodeGroups(nodeGroups, inProgressCNRs)
}

// observer the changes using the remaining nodegroups. This is stateless and will pickup changes again if restarted
changedNodeGroups := c.observeChanges(nodeGroups)
// observe the changes using the remaining nodegroups. This is stateless and will pickup changes again if restarted
changedNodeGroups := c.observeChanges(nodeGroups)
c.NodeGroupsDetectedChanges.WithLabelValues().Set(float64(len(changedNodeGroups)))
if len(changedNodeGroups) == 0 {
klog.V(2).Infoln("all nodegroups up to date. next check in", c.CheckInterval)
return
Expand All @@ -334,12 +381,31 @@ func (c *controller) Run() {
}
}

// wait for the desired amount to allow any in progress changes to batch up
// Filter to only the lowest priority nodegroups
filtered := c.filterNodeGroupsByPriority(changedNodeGroups)
if len(filtered) == 0 {
klog.V(2).Infoln("no nodegroups to apply after priority filter")
return
}

// If any lower priority CNRs are still in progress, skip this run
minP := max(filtered[0].NodeGroup.Spec.Priority, 0)
if c.isLowerPriorityInProgress(minP, inProgressCNRs) {
c.NodeGroupsBlocked.WithLabelValues().Set(float64(len(filtered)))
klog.V(2).Infof("lower priority CNRs still in progress for priority < %d; skipping creation", minP)
return
}
// Not blocked
c.NodeGroupsBlocked.WithLabelValues().Set(0)

// wait for the desired amount to allow any in progress changes to batch up
klog.V(3).Infof("waiting for %v to allow changes to settle", c.WaitInterval)
select {
case <-time.After(c.WaitInterval):
klog.V(3).Infof("applying %d CNRs", len(changedNodeGroups))
c.createCNRs(changedNodeGroups)
klog.V(3).Infof("applying %d CNRs (lowest priority batch)", len(filtered))
c.NodeGroupsApplying.WithLabelValues().Set(float64(len(filtered)))
c.createCNRs(filtered)
c.NodeGroupsApplying.WithLabelValues().Set(0)
if c.RunOnce {
klog.V(3).Infoln("done creating CNRs after runOnce. exiting")
} else {
Expand Down
Loading