Skip to content

Commit b715d6c

Browse files
committed
fix: re-price nodes after every pricing update
Previously there was a race between pricing updates and node creation. If the pricing updated first, you would get new pricing (OD & Spot) for nodes. If the nodes were created first, you would use stale OD pricing and not have spot pricing. This change forces all nodes to have their prices re-calculated after every pricing update.
1 parent ef83274 commit b715d6c

4 files changed

Lines changed: 48 additions & 31 deletions

File tree

cmd/eks-node-viewer/main.go

Lines changed: 11 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020
"flag"
2121
"fmt"
2222
"log"
23-
"math"
2423
"os"
2524
"strings"
2625
"time"
@@ -74,13 +73,18 @@ func main() {
7473

7574
defaults.SharedCredentialsFilename()
7675
pprov := pricing.NewStaticProvider()
76+
m := model.NewUIModel(strings.Split(flags.ExtraLabels, ","))
77+
m.SetResources(strings.FieldsFunc(flags.Resources, func(r rune) bool { return r == ',' }))
78+
7779
if !flags.DisablePricing {
7880
sess := session.Must(session.NewSession(nil))
79-
pprov = pricing.NewProvider(ctx, sess)
81+
updateAllPrices := func() {
82+
m.Cluster().ForEachNode(func(n *model.Node) {
83+
n.UpdatePrice(pprov)
84+
})
85+
}
86+
pprov = pricing.NewProvider(ctx, sess, updateAllPrices)
8087
}
81-
m := model.NewUIModel(strings.Split(flags.ExtraLabels, ","))
82-
83-
m.SetResources(strings.FieldsFunc(flags.Resources, func(r rune) bool { return r == ',' }))
8488

8589
var nodeSelector labels.Selector
8690
if ns, err := labels.Parse(flags.NodeSelector); err != nil {
@@ -126,7 +130,7 @@ func startMonitor(ctx context.Context, settings *monitorSettings) {
126130
node, ok := cluster.GetNode(p.Spec.NodeName)
127131
// need to potentially update node price as we need the fargate pod in order to figure out the cost
128132
if ok && node.IsFargate() && !node.HasPrice() {
129-
updatePrice(settings, node)
133+
node.UpdatePrice(settings.pricing)
130134
}
131135
}
132136
},
@@ -163,7 +167,7 @@ func startMonitor(ctx context.Context, settings *monitorSettings) {
163167
cache.ResourceEventHandlerFuncs{
164168
AddFunc: func(obj interface{}) {
165169
node := model.NewNode(obj.(*v1.Node))
166-
updatePrice(settings, node)
170+
node.UpdatePrice(settings.pricing)
167171
n := cluster.AddNode(node)
168172
n.Show()
169173
},
@@ -190,27 +194,6 @@ func startMonitor(ctx context.Context, settings *monitorSettings) {
190194

191195
}
192196

193-
func updatePrice(settings *monitorSettings, node *model.Node) {
194-
// lookup our node price
195-
node.Price = math.NaN()
196-
if node.IsOnDemand() {
197-
if price, ok := settings.pricing.OnDemandPrice(node.InstanceType()); ok {
198-
node.Price = price
199-
}
200-
} else if node.IsSpot() {
201-
if price, ok := settings.pricing.SpotPrice(node.InstanceType(), node.Zone()); ok {
202-
node.Price = price
203-
}
204-
} else if node.IsFargate() && len(node.Pods()) == 1 {
205-
cpu, mem, ok := node.Pods()[0].FargateCapacityProvisioned()
206-
if ok {
207-
if price, ok := settings.pricing.FargatePrice(cpu, mem); ok {
208-
node.Price = price
209-
}
210-
}
211-
}
212-
}
213-
214197
// isTerminalPod returns true if the pod is deleting or in a terminal state
215198
func isTerminalPod(p *v1.Pod) bool {
216199
if !p.DeletionTimestamp.IsZero() {

pkg/model/cluster.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,14 @@ func (c *Cluster) DeleteNode(name string) {
6565
}
6666
}
6767

68+
func (c *Cluster) ForEachNode(f func(n *Node)) {
69+
c.mu.RLock()
70+
defer c.mu.RUnlock()
71+
for _, n := range c.nodes {
72+
f(n)
73+
}
74+
}
75+
6876
func (c *Cluster) GetNode(name string) (*Node, bool) {
6977
c.mu.RLock()
7078
defer c.mu.RUnlock()
@@ -143,7 +151,7 @@ func (c *Cluster) Stats() Stats {
143151
}
144152
// only add the price if it's not NaN which is used to indicate an unknown
145153
// price
146-
if n.Price == n.Price {
154+
if n.HasPrice() {
147155
st.TotalPrice += n.Price
148156
}
149157
st.NumNodes++

pkg/model/node.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,13 @@ package model
1616

1717
import (
1818
"fmt"
19+
"math"
1920
"sync"
2021
"time"
2122

2223
v1 "k8s.io/api/core/v1"
24+
25+
"github.com/awslabs/eks-node-viewer/pkg/pricing"
2326
)
2427

2528
type objectKey struct {
@@ -207,3 +210,24 @@ func (n *Node) HasPrice() bool {
207210
// we use NaN for an unknown price, so if this is true the price is known
208211
return n.Price == n.Price
209212
}
213+
214+
func (n *Node) UpdatePrice(pricing *pricing.Provider) {
215+
// lookup our n price
216+
n.Price = math.NaN()
217+
if n.IsOnDemand() {
218+
if price, ok := pricing.OnDemandPrice(n.InstanceType()); ok {
219+
n.Price = price
220+
}
221+
} else if n.IsSpot() {
222+
if price, ok := pricing.SpotPrice(n.InstanceType(), n.Zone()); ok {
223+
n.Price = price
224+
}
225+
} else if n.IsFargate() && len(n.Pods()) == 1 {
226+
cpu, mem, ok := n.Pods()[0].FargateCapacityProvisioned()
227+
if ok {
228+
if price, ok := pricing.FargatePrice(cpu, mem); ok {
229+
n.Price = price
230+
}
231+
}
232+
}
233+
}

pkg/pricing/pricing.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ type Provider struct {
5454
spotPrices map[string]zonalPricing
5555
fargateVCPUPricePerHour float64
5656
fargateGBPricePerHour float64
57+
notify func()
5758
}
5859

5960
// zonalPricing is used to capture the per-zone price
@@ -97,7 +98,7 @@ func NewStaticProvider() *Provider {
9798
spotUpdateTime: initialPriceUpdate,
9899
}
99100
}
100-
func NewProvider(ctx context.Context, sess *session.Session) *Provider {
101+
func NewProvider(ctx context.Context, sess *session.Session, notify func()) *Provider {
101102
region := "us-west-2"
102103
if aws.StringValue(sess.Config.Region) != "" {
103104
region = aws.StringValue(sess.Config.Region)
@@ -110,6 +111,7 @@ func NewProvider(ctx context.Context, sess *session.Session) *Provider {
110111
spotUpdateTime: initialPriceUpdate,
111112
ec2: ec2.New(sess),
112113
pricing: NewPricingAPI(sess, region),
114+
notify: notify,
113115
}
114116

115117
go func() {
@@ -212,8 +214,8 @@ func (p *Provider) updatePricing(ctx context.Context) {
212214
log.Printf("updating fargate pricing, %s", err)
213215
}
214216
}()
215-
216217
wg.Wait()
218+
p.notify()
217219
}
218220

219221
func (p *Provider) updateOnDemandPricing(ctx context.Context) error {

0 commit comments

Comments
 (0)