-
Notifications
You must be signed in to change notification settings - Fork 344
chore(IDX): k8s config updates #4815
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
base: master
Are you sure you want to change the base?
Conversation
Co-authored-by: Bas van Dijk <bas@dfinity.org>
rs/tests/driver/src/k8s/tnet.rs
Outdated
// We make reservation for 42 vcpus only if VM uses 64 vcpus because there are | ||
// already other k8s resources having resource requests that prevents reservation to succeeds. | ||
// Note that VM still gets 64 vcpus. | ||
let vcpus = min(36, vm_req.vcpus.get()).to_string(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we can't reserve more than 42 vCPUs shouldn't we have:
let vcpus = min(36, vm_req.vcpus.get()).to_string(); | |
let vcpus = max(42, vm_req.vcpus.get()).to_string(); |
rs/tests/driver/src/k8s/tnet.rs
Outdated
@@ -389,11 +398,19 @@ impl TNet { | |||
// TODO: only download it once and copy it if it's already downloaded | |||
let args = format!( | |||
"set -xe; \ | |||
apk add file; \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the below piece of code shouldn't we check if the hash of the downloaded file matches what we expect (like we do in Farm)?
No description provided.