fix: Modify workflow to use new eks models - Release 0.7.0 #78
Conversation
mbeacom
changed the title
mbeacom
changed the title
Codecov ReportPatch coverage:
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more Additional details and impacted files@@ Coverage Diff @@
## main #78 +/- ##
==========================================
+ Coverage 21.03% 27.40% +6.36%
==========================================
Files 12 13 +1
Lines 1740 1927 +187
==========================================
+ Hits 366 528 +162
- Misses 1374 1399 +25
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
mbeacom
changed the title
|
Hello @mbeacom AND Any idea when this PR will be merged? I'll wait to upgrade from 1.23 to 1.24 to see if the add-ons portion has a new behavior then. Thanks again for all your efforts in making this tool AMAZING! |
mbeacom
changed the title
mbeacom
changed the title
|
@onabison Thanks for the kind words! Yes, this PR does indeed resolve the mentioned issue. This should be merged (barring any major issues discovered in review) today or tomorrow. |
|
v0.7.0
|
quixoticmonk
force-pushed
the
modified-workflow
branch
from
6c8b0c3 to
8799724
Compare
| @@ -97,23 +401,26 @@ class ClusterAddon(EksResource): | |||
| product_id: str = "" | |||
| product_url: str = "" | |||
| configuration_values: str = "" | |||
| cluster: Cluster = field(default_factory=lambda: Cluster(arn="")) | |||
| cluster: Cluster = field(default_factory=lambda: Cluster(arn="", version="1.24")) | |||
There was a problem hiding this comment.
is this default version necessary/required? is there any possibility of someone updating a cluster and this is used instead of the control plane's current version?
There was a problem hiding this comment.
Will adjust the init workflow for this to eliminate the requirement. It shouldn't ever occur within the scripted workflow, but (to your point) could absolutely cause version confusion if used programmatically (and in other custom workflows).
I will modify the version parsing so this shortcut isn't required in my follow-up PR with parallel execs.
| upgrade_details[addon.name] = _update_responses | ||
| return upgrade_details | ||
|
|
||
| def upgrade_nodegroups(self, wait: bool = False) -> Dict[str, Any]: |
There was a problem hiding this comment.
I wonder if a parallel flag should be propagated down to here. some customers have a nodegroup per team which results in several (up to 100 - yes some are at that limit) nodegroups. on one hand it speeds things up, but on the other, it could be asking for trouble depending on how many nodes in the cluster are churning at once
There was a problem hiding this comment.
You're describing my current stash against this branch 😂 -- added parallel propagation through to the nodegroup updates that trigger collection of all the futures and subsequently gather results, but figured I should wait until the next release to introduce it. Happy to include it here, if desired.
There was a problem hiding this comment.
ya I think a follow up PR is fine
| """Upgrade all cluster addons.""" | ||
| logger.info("The add-ons update has been initiated...") | ||
| upgrade_details: Dict[str, Any] = {} | ||
| for addon in self.upgradable_addons: |
There was a problem hiding this comment.
these can definitely be done in parallel FYI
There was a problem hiding this comment.
+1 I was likely going to throw them into a PoolExecutor and execute them concurrently and gather results off of the pool.
Co-authored-by: Bryant Biggs <[email protected]>
Co-authored-by: Bryant Biggs <[email protected]>
|
@onabison |
Summary
Resolves: #69 #36
Changes
This PR modifies the current eksupgrade tool workflows to use the new model-based implementation. This leans on the AWS API to perform the majority of upgrade steps when possible (cluster, addons, managed node groups), instead of manually handling them through custom logic.
User experience
The user experience is altered as a result of this PR. The user can now totally disable validation checks via
--disable-checksand install the latest add-ons during upgrade via--latest-addonsin order to update addons beyond thedefaultto the latest available.Additionally, these changes now employ boto3 waiters to intelligently poll for resource status updates, as opposed to hand rolling the waiters or minimally
sleeping throughout the workflows.Checklist
If your change doesn't seem to apply, please leave them unchecked.
Acknowledgment
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.