[arvados] updated: 2.5.0-10-g6a9efc57d
git repository hosting
git at public.arvados.org
Tue Jan 24 23:08:44 UTC 2023
Summary of changes:
lib/cloud/azure/azure.go | 2 +-
lib/cloud/ec2/ec2.go | 32 +++++++++++++++++++++++---------
lib/cloud/ec2/ec2_test.go | 16 ++++++++++++----
lib/cloud/interfaces.go | 7 +++++--
lib/cloud/loopback/loopback.go | 14 +++++++-------
lib/config/config.default.yml | 18 +++++++++++++++++-
lib/crunchrun/crunchrun.go | 7 +++++++
lib/crunchrun/crunchrun_test.go | 3 +++
lib/dispatchcloud/test/stub_driver.go | 2 +-
lib/dispatchcloud/worker/worker.go | 2 +-
10 files changed, 77 insertions(+), 26 deletions(-)
via 6a9efc57d57968d7504327c0569a3acab3925db3 (commit)
via 99a4dc213a58af8eb019ca270b0982286beeb5a2 (commit)
via 36a6bcdb43e4add604bd8ac6409eda6abc248c8c (commit)
via 48697cdb28fca37f1e420855a1bf1af2446184c7 (commit)
from c138f58b21edb574b101588f6fc61dce8a98ed3e (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
commit 6a9efc57d57968d7504327c0569a3acab3925db3
Author: Tom Clegg <tom at curii.com>
Date: Tue Jan 24 18:04:07 2023 -0500
19320: Deduplicate instance types in spot price request.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/lib/cloud/ec2/ec2.go b/lib/cloud/ec2/ec2.go
index a4f6b2da7..b90eff6d5 100644
--- a/lib/cloud/ec2/ec2.go
+++ b/lib/cloud/ec2/ec2.go
@@ -337,7 +337,8 @@ func (instanceSet *ec2InstanceSet) updateSpotPrices(instances []cloud.Instance)
updateTime := time.Now()
staleTime := updateTime.Add(-instanceSet.ec2config.SpotPriceUpdateInterval.Duration())
needUpdate := false
- var typeFilterValues []*string
+ allTypes := map[string]bool{}
+
for _, inst := range instances {
ec2inst := inst.(*ec2Instance).instance
if aws.StringValue(ec2inst.InstanceLifecycle) == "spot" {
@@ -349,12 +350,16 @@ func (instanceSet *ec2InstanceSet) updateSpotPrices(instances []cloud.Instance)
if instanceSet.pricesUpdated[pk].Before(staleTime) {
needUpdate = true
}
- typeFilterValues = append(typeFilterValues, ec2inst.InstanceType)
+ allTypes[*ec2inst.InstanceType] = true
}
}
if !needUpdate {
return
}
+ var typeFilterValues []*string
+ for instanceType := range allTypes {
+ typeFilterValues = append(typeFilterValues, aws.String(instanceType))
+ }
// Get 3x update interval worth of pricing data. (Ideally the
// AWS API would tell us "we have shown you all of the price
// changes up to time T", but it doesn't, so we'll just ask
commit 99a4dc213a58af8eb019ca270b0982286beeb5a2
Author: Tom Clegg <tom at curii.com>
Date: Tue Jan 24 17:44:27 2023 -0500
19320: Comment re future use of spot attr in priceKey.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/lib/cloud/ec2/ec2.go b/lib/cloud/ec2/ec2.go
index 7c7c48b9b..a4f6b2da7 100644
--- a/lib/cloud/ec2/ec2.go
+++ b/lib/cloud/ec2/ec2.go
@@ -506,6 +506,9 @@ func (inst *ec2Instance) VerifyHostKey(ssh.PublicKey, *ssh.Client) error {
func (inst *ec2Instance) PriceHistory(instType arvados.InstanceType) []cloud.InstancePrice {
inst.provider.pricesLock.Lock()
defer inst.provider.pricesLock.Unlock()
+ // Note updateSpotPrices currently populates
+ // inst.provider.prices only for spot instances, so if
+ // spot==false here, we will return no data.
pk := priceKey{
instanceType: *inst.instance.InstanceType,
spot: aws.StringValue(inst.instance.InstanceLifecycle) == "spot",
commit 36a6bcdb43e4add604bd8ac6409eda6abc248c8c
Author: Tom Clegg <tom at curii.com>
Date: Tue Jan 24 17:40:34 2023 -0500
19320: Disable spot price checks if configured update interval <= 0.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/lib/cloud/ec2/ec2.go b/lib/cloud/ec2/ec2.go
index 93c308bcb..7c7c48b9b 100644
--- a/lib/cloud/ec2/ec2.go
+++ b/lib/cloud/ec2/ec2.go
@@ -246,10 +246,6 @@ func (instanceSet *ec2InstanceSet) Create(
}
}
- if instanceSet.ec2config.SpotPriceUpdateInterval <= 0 {
- instanceSet.ec2config.SpotPriceUpdateInterval = arvados.Duration(24 * time.Hour)
- }
-
rsv, err := instanceSet.client.RunInstances(&rii)
err = wrapError(err, &instanceSet.throttleDelayCreate)
if err != nil {
@@ -296,7 +292,7 @@ func (instanceSet *ec2InstanceSet) Instances(tags cloud.InstanceTags) (instances
}
dii.NextToken = dio.NextToken
}
- if needAZs {
+ if needAZs && instanceSet.ec2config.SpotPriceUpdateInterval > 0 {
az := map[string]string{}
err := instanceSet.client.DescribeInstanceStatusPages(&ec2.DescribeInstanceStatusInput{
IncludeAllInstances: aws.Bool(true),
diff --git a/lib/cloud/ec2/ec2_test.go b/lib/cloud/ec2/ec2_test.go
index 79ee0d04e..2b4b19850 100644
--- a/lib/cloud/ec2/ec2_test.go
+++ b/lib/cloud/ec2/ec2_test.go
@@ -191,7 +191,9 @@ func GetInstanceSet(c *check.C) (*ec2InstanceSet, cloud.ImageID, arvados.Cluster
return ap.(*ec2InstanceSet), cloud.ImageID(exampleCfg.ImageIDForTestSuite), cluster
}
ap := ec2InstanceSet{
- ec2config: ec2InstanceSetConfig{},
+ ec2config: ec2InstanceSetConfig{
+ SpotPriceUpdateInterval: arvados.Duration(time.Hour),
+ },
instanceSetID: "test123",
logger: logrus.StandardLogger(),
client: &ec2stub{c: c, reftime: time.Now().UTC()},
diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml
index 078dbf494..f6cf0c91a 100644
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -1397,7 +1397,8 @@ Clusters:
# (ec2) how often to look up spot instance pricing data
# (only while running spot instances) for the purpose of
- # calculating container cost estimates.
+ # calculating container cost estimates. A value of 0
+ # disables spot price lookups entirely.
SpotPriceUpdateInterval: 24h
# (ec2) per-GiB-month cost of EBS volumes. Matches
commit 48697cdb28fca37f1e420855a1bf1af2446184c7
Author: Tom Clegg <tom at curii.com>
Date: Tue Jan 24 17:31:33 2023 -0500
19320: Account for AddedScratch in spot instance cost estimates.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/lib/cloud/azure/azure.go b/lib/cloud/azure/azure.go
index c27800592..7b170958b 100644
--- a/lib/cloud/azure/azure.go
+++ b/lib/cloud/azure/azure.go
@@ -785,7 +785,7 @@ func (ai *azureInstance) Address() string {
}
}
-func (ai *azureInstance) PriceHistory() []cloud.InstancePrice {
+func (ai *azureInstance) PriceHistory(arvados.InstanceType) []cloud.InstancePrice {
return nil
}
diff --git a/lib/cloud/ec2/ec2.go b/lib/cloud/ec2/ec2.go
index 2a5eea484..93c308bcb 100644
--- a/lib/cloud/ec2/ec2.go
+++ b/lib/cloud/ec2/ec2.go
@@ -48,6 +48,7 @@ type ec2InstanceSetConfig struct {
SubnetID string
AdminUsername string
EBSVolumeType string
+ EBSPrice float64
IAMInstanceProfile string
SpotPriceUpdateInterval arvados.Duration
}
@@ -506,7 +507,7 @@ func (inst *ec2Instance) VerifyHostKey(ssh.PublicKey, *ssh.Client) error {
// Spot price that is in effect when your Spot Instance is running."
// (The use of the phrase "is running", as opposed to "was launched",
// hints that pricing is dynamic.)
-func (inst *ec2Instance) PriceHistory() []cloud.InstancePrice {
+func (inst *ec2Instance) PriceHistory(instType arvados.InstanceType) []cloud.InstancePrice {
inst.provider.pricesLock.Lock()
defer inst.provider.pricesLock.Unlock()
pk := priceKey{
@@ -514,7 +515,16 @@ func (inst *ec2Instance) PriceHistory() []cloud.InstancePrice {
spot: aws.StringValue(inst.instance.InstanceLifecycle) == "spot",
availabilityZone: inst.availabilityZone,
}
- return inst.provider.prices[pk]
+ var prices []cloud.InstancePrice
+ for _, price := range inst.provider.prices[pk] {
+ // ceil(added scratch space in GiB)
+ gib := (instType.AddedScratch + 1<<30 - 1) >> 30
+ monthly := inst.provider.ec2config.EBSPrice * float64(gib)
+ hourly := monthly / 30 / 24
+ price.Price += hourly
+ prices = append(prices, price)
+ }
+ return prices
}
type rateLimitError struct {
diff --git a/lib/cloud/ec2/ec2_test.go b/lib/cloud/ec2/ec2_test.go
index e7534a7b6..79ee0d04e 100644
--- a/lib/cloud/ec2/ec2_test.go
+++ b/lib/cloud/ec2/ec2_test.go
@@ -150,7 +150,7 @@ func (e *ec2stub) TerminateInstances(input *ec2.TerminateInstancesInput) (*ec2.T
return nil, nil
}
-func GetInstanceSet(c *check.C) (cloud.InstanceSet, cloud.ImageID, arvados.Cluster) {
+func GetInstanceSet(c *check.C) (*ec2InstanceSet, cloud.ImageID, arvados.Cluster) {
cluster := arvados.Cluster{
InstanceTypes: arvados.InstanceTypeMap(map[string]arvados.InstanceType{
"tiny": {
@@ -188,7 +188,7 @@ func GetInstanceSet(c *check.C) (cloud.InstanceSet, cloud.ImageID, arvados.Clust
ap, err := newEC2InstanceSet(exampleCfg.DriverParameters, "test123", nil, logrus.StandardLogger())
c.Assert(err, check.IsNil)
- return ap, cloud.ImageID(exampleCfg.ImageIDForTestSuite), cluster
+ return ap.(*ec2InstanceSet), cloud.ImageID(exampleCfg.ImageIDForTestSuite), cluster
}
ap := ec2InstanceSet{
ec2config: ec2InstanceSetConfig{},
@@ -297,6 +297,7 @@ func (*EC2InstanceSetSuite) TestInstancePriceHistory(c *check.C) {
}
}()
+ ap.ec2config.EBSPrice = 0.1 // $/GiB/month
inst1, err := ap.Create(cluster.InstanceTypes["tiny-preemptible"], img, tags, "true", pk)
c.Assert(err, check.IsNil)
defer inst1.Destroy()
@@ -328,14 +329,19 @@ func (*EC2InstanceSetSuite) TestInstancePriceHistory(c *check.C) {
}
for _, inst := range instances {
- hist := inst.PriceHistory()
+ hist := inst.PriceHistory(arvados.InstanceType{})
c.Logf("%s price history: %v", inst.ID(), hist)
c.Check(len(hist) > 0, check.Equals, true)
+
+ histWithScratch := inst.PriceHistory(arvados.InstanceType{AddedScratch: 640 << 30})
+ c.Logf("%s price history with 640 GiB scratch: %v", inst.ID(), histWithScratch)
+
for i, ip := range hist {
c.Check(ip.Price, check.Not(check.Equals), 0.0)
if i > 0 {
c.Check(ip.StartTime.Before(hist[i-1].StartTime), check.Equals, true)
}
+ c.Check(ip.Price < histWithScratch[i].Price, check.Equals, true)
}
}
}
diff --git a/lib/cloud/interfaces.go b/lib/cloud/interfaces.go
index 7f5904968..27cf26152 100644
--- a/lib/cloud/interfaces.go
+++ b/lib/cloud/interfaces.go
@@ -102,8 +102,11 @@ type Instance interface {
// Replace tags with the given tags
SetTags(InstanceTags) error
- // Get recent price history, if available
- PriceHistory() []InstancePrice
+ // Get recent price history, if available. The InstanceType is
+ // supplied as an argument so the driver implementation can
+ // account for AddedScratch cost without requesting the volume
+ // attachment information from the provider's API.
+ PriceHistory(arvados.InstanceType) []InstancePrice
// Shut down the node
Destroy() error
diff --git a/lib/cloud/loopback/loopback.go b/lib/cloud/loopback/loopback.go
index ed2a0050f..8afaa4525 100644
--- a/lib/cloud/loopback/loopback.go
+++ b/lib/cloud/loopback/loopback.go
@@ -130,13 +130,13 @@ type instance struct {
sshService test.SSHService
}
-func (i *instance) ID() cloud.InstanceID { return cloud.InstanceID(i.instanceType.ProviderType) }
-func (i *instance) String() string { return i.instanceType.ProviderType }
-func (i *instance) ProviderType() string { return i.instanceType.ProviderType }
-func (i *instance) Address() string { return i.sshService.Address() }
-func (i *instance) PriceHistory() []cloud.InstancePrice { return nil }
-func (i *instance) RemoteUser() string { return i.adminUser }
-func (i *instance) Tags() cloud.InstanceTags { return i.tags }
+func (i *instance) ID() cloud.InstanceID { return cloud.InstanceID(i.instanceType.ProviderType) }
+func (i *instance) String() string { return i.instanceType.ProviderType }
+func (i *instance) ProviderType() string { return i.instanceType.ProviderType }
+func (i *instance) Address() string { return i.sshService.Address() }
+func (i *instance) PriceHistory(arvados.InstanceType) []cloud.InstancePrice { return nil }
+func (i *instance) RemoteUser() string { return i.adminUser }
+func (i *instance) Tags() cloud.InstanceTags { return i.tags }
func (i *instance) SetTags(tags cloud.InstanceTags) error {
i.tags = tags
return nil
diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml
index a8adaeff8..078dbf494 100644
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -1400,6 +1400,14 @@ Clusters:
# calculating container cost estimates.
SpotPriceUpdateInterval: 24h
+ # (ec2) per-GiB-month cost of EBS volumes. Matches
+ # EBSVolumeType. Used to account for AddedScratch when
+ # calculating container cost estimates. Note that
+ # https://aws.amazon.com/ebs/pricing/ defines GB to mean
+ # GiB, so an advertised price $0.10/GB indicates a real
+ # price of $0.10/GiB and can be entered here as 0.10.
+ EBSPrice: 0.10
+
# (azure) Credentials.
SubscriptionID: ""
ClientID: ""
@@ -1453,6 +1461,13 @@ Clusters:
RAM: 128MiB
IncludedScratch: 16GB
AddedScratch: 0
+ # Hourly price ($), used to select node types for containers,
+ # and to calculate estimated container costs. For spot
+ # instances on EC2, this is also used as the maximum price
+ # when launching spot instances, while the estimated container
+ # cost is computed based on the current spot price according
+ # to AWS. On Azure, and on-demand instances on EC2, the price
+ # given here is used to compute container cost estimates.
Price: 0.1
Preemptible: false
# Include this section if the node type includes GPU (CUDA) support
diff --git a/lib/crunchrun/crunchrun.go b/lib/crunchrun/crunchrun.go
index 1f130a5b1..507dfc4dd 100644
--- a/lib/crunchrun/crunchrun.go
+++ b/lib/crunchrun/crunchrun.go
@@ -2298,6 +2298,12 @@ func (cr *ContainerRunner) calculateCost(now time.Time) float64 {
spanEnd := now
for _, ip := range prices {
spanStart := ip.StartTime
+ if spanStart.After(now) {
+ // pricing information from the future -- not
+ // expected from AWS, but possible in
+ // principle, and exercised by tests.
+ continue
+ }
last := false
if spanStart.Before(cr.costStartTime) {
spanStart = cr.costStartTime
@@ -2309,5 +2315,6 @@ func (cr *ContainerRunner) calculateCost(now time.Time) float64 {
}
spanEnd = spanStart
}
+
return cost
}
diff --git a/lib/crunchrun/crunchrun_test.go b/lib/crunchrun/crunchrun_test.go
index a5045863b..f4cd6e609 100644
--- a/lib/crunchrun/crunchrun_test.go
+++ b/lib/crunchrun/crunchrun_test.go
@@ -2119,6 +2119,9 @@ func (s *TestSuite) TestCalculateCost(c *C) {
cost = cr.calculateCost(now)
c.Check(cost, Equals, 1.0/2+2.0/4+3.0/4)
+ cost = cr.calculateCost(now.Add(-time.Hour / 2))
+ c.Check(cost, Equals, 0.5)
+
c.Logf("%s", logbuf.String())
c.Check(logbuf.String(), Matches, `(?ms).*Instance price changed to 1\.00 at 20.* changed to 2\.00 .* changed to 3\.00 .*`)
c.Check(logbuf.String(), Not(Matches), `(?ms).*changed to 2\.00 .* changed to 2\.00 .*`)
diff --git a/lib/dispatchcloud/test/stub_driver.go b/lib/dispatchcloud/test/stub_driver.go
index bb134e454..01af8e6d5 100644
--- a/lib/dispatchcloud/test/stub_driver.go
+++ b/lib/dispatchcloud/test/stub_driver.go
@@ -471,6 +471,6 @@ func copyTags(src cloud.InstanceTags) cloud.InstanceTags {
return dst
}
-func (si stubInstance) PriceHistory() []cloud.InstancePrice {
+func (si stubInstance) PriceHistory(arvados.InstanceType) []cloud.InstancePrice {
return nil
}
diff --git a/lib/dispatchcloud/worker/worker.go b/lib/dispatchcloud/worker/worker.go
index 397a46292..b2ed6c2bf 100644
--- a/lib/dispatchcloud/worker/worker.go
+++ b/lib/dispatchcloud/worker/worker.go
@@ -384,7 +384,7 @@ func (wkr *worker) probeRunning() (running []string, reportsBroken, ok bool) {
}
before := time.Now()
var stdin io.Reader
- if prices := wkr.instance.PriceHistory(); len(prices) > 0 {
+ if prices := wkr.instance.PriceHistory(wkr.instType); len(prices) > 0 {
j, _ := json.Marshal(prices)
stdin = bytes.NewReader(j)
}
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list