Improve AWS Chaos #4085
Improve AWS Chaos #4085
Conversation
miketonks-form3
force-pushed
the
mike-aws-chaos
branch
from
2279cd2
to
147057d
Compare
pkg/selector/aws/selector.go
Outdated
| }))) | ||
| } | ||
|
|
||
| // TODO How to get namespace for secret?? |
There was a problem hiding this comment.
I could use some help here. I can't see how to get the namespace in the selector module.
It seems like the pod selector must do this, but I couldn't really follow the code.
There was a problem hiding this comment.
we assume that the secret is in the same namespace with the AWSChaos, so we could use the namespace of AWSChaos.
Is it OK?
There was a problem hiding this comment.
Yes, it seems to be good 👍
miketonks-form3
force-pushed
the
mike-aws-chaos
branch
from
2ccea4f
to
8945d83
Compare
pkg/selector/aws/selector.go
Outdated
| type SelectImpl struct { | ||
| c client.Client | ||
| generic.Option | ||
| } | ||
|
|
||
| func (impl *SelectImpl) Select(ctx context.Context, awsSelector *v1alpha1.AWSSelector) ([]*v1alpha1.AWSSelector, error) { |
There was a problem hiding this comment.
It seems that we could use an other type instead of []*v1alpha1.AWSSelector in the return value as the "selected records". v1alpha1.AWSSelector carries too much noise info that we do not need in the next steps.
How about creating a new struct to introduce the selected ec2 or volume info?
There was a problem hiding this comment.
Yes, I agree it's better. I added aws.Instance struct type for this. Thanks!
miketonks-form3
force-pushed
the
mike-aws-chaos
branch
from
a2e143f
to
82df9b3
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4085 +/- ##
==========================================
+ Coverage 38.68% 38.72% +0.03%
==========================================
Files 169 170 +1
Lines 13950 14057 +107
==========================================
+ Hits 5397 5443 +46
- Misses 8105 8163 +58
- Partials 448 451 +3
... and 4 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
|
Fixed a few small issues, we are now running the build on our CI and everything is passing. Could someone take a look at this? |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
kenneth-buck-form3
force-pushed
the
mike-aws-chaos
branch
2 times, most recently
from
7d1ad74
to
6e47a78
Compare
|
Hello, I am one of the people that is picking up this PR to help see it through. I updated the branch and fixed any missing signed-by commits. Is there anything else that I can do in order to help land this in master? Thank you in advance :) |
Hi @kenneth-buck-form3, thank you very much for your work! We are going to go review the recent changes this week (Possibly at the weekend). |
miketonks-form3
force-pushed
the
mike-aws-chaos
branch
from
6e47a78
to
2e712e0
Compare
miketonks-form3
force-pushed
the
mike-aws-chaos
branch
from
2e712e0
to
8c38ee7
Compare
|
Hi, this PR is quite old now. I have updated it with latest changes. Are we still waiting for a review @g1eny0ung ? I'm not sure why the tests failed on previous run, I think it may be unrelated to my changes. Hoping that it will pass with latest updates from master. |
Signed-off-by: Mike Tonks <michael.tonks@form3.tech> Signed-off-by: Kenneth Buck <kenneth.buck@form3.tech>
Signed-off-by: Graham Brereton <graham.brereton@form3.tech>
grahambrereton-form3
force-pushed
the
mike-aws-chaos
branch
from
6c03356
to
d43214d
Compare
|
👋 Any news about it ? :) |
What problem does this PR solve?
Extends the ASWChaos type to allow selecting instances using filters.
Work towards: #4082
What's changed and how it works?
Proposal: RFC: Improve Cloud Chaos Providers
Related changes
UI interfaceCherry-pick to release branches (optional)
Checklist
CHANGELOG
CHANGELOG.mdTests
Side effects
DCO
If you find the DCO check fails, please run commands like below (Depends on the actual situations. For example, if the failed commit isn't the most recent) to fix it: