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

revert change to increase min. CPU resource value from 20 to 100 #3706

Merged
merged 1 commit into from
Jan 3, 2018
Merged

revert change to increase min. CPU resource value from 20 to 100 #3706

merged 1 commit into from
Jan 3, 2018

Conversation

fho
Copy link
Contributor

@fho fho commented Jan 2, 2018

In the commit 622d3dd
"Fixed test and moved constants into standalone func" the minimum CPU
resource value for a job was increased from 100 to 20.

This can break the nomad setup for people that used lower CPU
values and are at the maximum MHz value of the available CPU on a
machine.
Change the minimum back to 20 MHz to ensure downwards compatibility.

In the commit 622d3dd
"Fixed test and moved constants into standalone func" the minimum CPU
resource value for a job was increased from 100 to 20.

This can break the nomad setup for people that used lower CPU
values and are at the maximum MHz value of the available CPU on a
machine.
Change the minimum back to 20 MHz to ensure downwards compatibility.
@fho
Copy link
Contributor Author

fho commented Jan 2, 2018

which test is failing? I can't find the information in the huge travis CI output

@preetapan
Copy link
Member

Two tests are failing in the Darwin test suite, these are known flaky tests that is on our list of tests to improve.

--- FAIL: TestNomad_BadExpect (81.06s)

	serf_test.go:339: should have 0 peers: 2

--

--- FAIL: TestServer_HeartbeatTTL_Failover (0.16s)

	heartbeat_test.go:244: err: leadership lost while committing log

@preetapan preetapan merged commit e0f84fc into hashicorp:master Jan 3, 2018
@preetapan
Copy link
Member

Thanks for catching this and submitting the fix @fho. Apologies for breaking your set up. I have merged this so the next release should have the old default.

@fho fho deleted the revert_min_cpu_limit_change branch January 3, 2018 17:46
@sirkjohannsen
Copy link

thank you so much @fho and @preetapan

paddycarver added a commit to hashicorp/terraform-provider-nomad that referenced this pull request Jan 29, 2018
Nomad 0.7.1 had an accidental backwards compatibility break and
increased the minimum CPU allocation allowed in the job spec. What was
previously 20 is now 100. This was reported at hashicorp/nomad#3709.

This breaks our tests, which had a CPU allocation of 20. This PR just
increases them to 100, which allows the tests to pass again.

A fix (hashicorp/nomad#3706) has been merged, and will be part of the
next release, but I think it's worthwhile to change our tests and avoid
contributors stumbling into this on their own when running against
0.7.1, even if it'll work again in the next version of Nomad.
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants