Host Update API integration #35
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
etungsten
force-pushed
the
update-api-integration
branch
from
4e7fd4f
to
77aa8dc
Compare
jahkeup
reviewed
Jul 10, 2020
samuelkarp
reviewed
Jul 10, 2020
pkg/agent/agent.go
Outdated
| // Get the updater interface version from the node label | ||
| var platformVersion = node.Labels[marker.UpdaterInterfaceVersionKey] | ||
| switch platformVersion { | ||
| case "2.0.0": |
There was a problem hiding this comment.
Do we want to use exactly 2.0.0, or a semver-compatible range more akin to 2.x.y?
There was a problem hiding this comment.
I don't see why not, but I also don't really see why we should. The platform version here is to differentiate hosts that can support the update API or not. There's really nothing much in between.
I think ideally the label would be more explicit. Instead of a version, it would just be the supported update platform, "updog" or "update-api".
pkg/agent/agent.go
Outdated
| default: | ||
| log.Info("unknown platform version specified, defaulting to using updog") | ||
| fallthrough | ||
| case "1.0.0": |
There was a problem hiding this comment.
Same question as above, do we want exactly 1.0.0?
jahkeup
reviewed
Jul 10, 2020
etungsten
force-pushed
the
update-api-integration
branch
2 times, most recently
from
a8893ea
to
44d3078
Compare
|
Push above addresses @samuelkarp and @jahkeup 's comments:
|
|
Testing wise, I got a node to update via the operator using the update API. However, I'm running into an issue where the agent repeatedly tries to prepare the update even after each attempt succeeds. The agent spins like this for a while until it finally realizes the update is ready then finally See below: Retrieves the available update fine, Then the agent tries to But then it goes back to posting the same intent again? It tries to re- Then finally the agent decides to Not entirely sure why this happens. I suspect it has something to with me sleeping to wait for the Logs from another attempt:
|
I don't think it is the sleep causing issue. The main event loop shouldn't be
We've seen similiar behavior with similiar messages. This makes me think it
When this happens, the Kubernetes control plane is literally rejecting the I think Reading over the rest of the func (a *Agent) postIntent(in *intent.Intent) error {
err := a.poster.Post(in)
if err == nil {
// track the successfully updated intent.
a.tracker.recordPost(in)
}
return err
}It's worth trying again with that method updated to see if the looping subsides. Also, if you are able to get the controller's logs at the same time that you saw I'll try out updating a cluster myself, though probably not until later today or |
Fixes postIntent logic so it properly records the intent if no error happened
etungsten
force-pushed
the
update-api-integration
branch
from
44d3078
to
ccf3431
Compare
|
Fixed the issue described in #35 (comment) thanks to @jahkeup 's suggestion. Now the node properly updates without spinning in place: |
samuelkarp
reviewed
Jul 15, 2020
etungsten
force-pushed
the
update-api-integration
branch
from
ccf3431
to
e56a9cd
Compare
|
Addresses @samuelkarp 's comments. |
|
Tested the latest changes and they still work. |
jahkeup
suggested changes
Jul 20, 2020
etungsten
force-pushed
the
update-api-integration
branch
from
e56a9cd
to
edf81ed
Compare
|
Push above addresses @jahkeup 's comments. Merged the two config files for deploying the operator. The diff with the original |
samuelkarp
reviewed
Jul 21, 2020
etungsten
force-pushed
the
update-api-integration
branch
from
edf81ed
to
e2cc062
Compare
|
Addresses @samuelkarp 's comments. Added a comment above the 10 second timeout when creating the http client (forgot to do this in the previous set of changes, oops!) Push below fixes a grammar error. |
etungsten
force-pushed
the
update-api-integration
branch
from
e2cc062
to
0d65f72
Compare
webern
approved these changes
Jul 21, 2020
|
|
||
| func TestUnmarshallUpdateStatus(t *testing.T) { | ||
| update_string := "Starting update to 0.4.0\n" | ||
| cases := []struct { |
There was a problem hiding this comment.
Wow, nice work creating these test cases.
etungsten
force-pushed
the
update-api-integration
branch
from
0d65f72
to
6c655c0
Compare
|
Push above addresses #35 (comment) |
jahkeup
reviewed
Jul 22, 2020
etungsten
force-pushed
the
update-api-integration
branch
from
6c655c0
to
69da3d8
Compare
|
Addresses @jahkeup's comments. |
samuelkarp
approved these changes
Jul 22, 2020
Adds support for updating via the Bottlerocket Update API. The new platform is used if the node is labeled bottlerocket.aws/updater-interface-version=2.0.0
etungsten
force-pushed
the
update-api-integration
branch
from
69da3d8
to
c2df97a
Compare
|
Push above fixes formatting. |
samuelkarp
approved these changes
Jul 22, 2020
jahkeup
approved these changes
Jul 22, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue number:
N/A
Description of changes:
Adds support for updating via the Bottlerocket Update API.
The new platform is used if the node is labeled
bottlerocket.aws/updater-interface-version=2.0.0Testing done:
Agent successfully scheduled a pod onto a node and finished an update:
Terms of contribution:
By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.