Grant S3 access to notebook & dask pods

[yuvipanda]

eksctl
supports
creating kubernetes service acocunts bound with
IRSA.
We create one with S3 access, and bind it to our notebook and dask
pods. This should give them full s3 access.

Remove separate eksctl cluster jsonnet object, since it was
not doing anything useful.

Stolen from #436

Fixes #492

[yuvipanda]

I've deployed this

[choldgraf]

Question here related to 2i2c-org/team-compass#175 - since you've already deployed this PR, what kind of feedback are you looking for in a review? Is there something specific you think needs eyeballs?

More generally, my instinct is that we should keep "merge" as an automatic step after "deploy". If something is deployed but not merged, then there's a mismatch between the state of the infrastructure and the state of our repositories, right? If that's the case, my feeling is that our policy should be "ask for reviews before deploying (if it's the kind of change that requires reviews), and once deployed, immediately merge"

[consideRatio]

LGTM from what I can tell! Novice to jsonnet syntax though.

[yuvipanda]

@choldgraf good question. So this was deployed, but @jhamman tested it and found bugs. so I'll amend this PR until it works and then merge it. This is similar to how it played out in #605 - @consideRatio provided valuble feedback that changed the PR before it was deployed.

So I've amended and deployed this PR again, and am waiting for Joe to test this again.

More generally, my instinct is that we should keep "merge" as an automatic step after "deploy". If something is deployed but not merged, then there's a mismatch between the state of the infrastructure and the state of our repositories, right? If that's the case, my feeling is that our policy should be "ask for reviews before deploying (if it's the kind of change that requires reviews), and once deployed, immediately merge"

I completely agree! However, I think for deployments reviews will not catch a lot of issues until after deployment, since we don't actually know how things will turn out until the deployment happens. Deploying is a bit like running code you write locally - it feels kinda wrong to ask people to review it without making any deploys, since it's like asking people to review code you haven't run locally...

So I'm not actually sure what the right thing to do here is, and I seem to be changing my mind here many times.

[yuvipanda]

Thanks, @consideRatio.

@choldgraf I agree that me merging this after I deployed it was probably the right thing to do - but I wanted to make sure I could both get @jhamman to test it and @consideRatio to review it. Some sort of post-deploy (not necessarily post-merge?) review...

[damianavila]

I completely agree! However, I think for deployments reviews will not catch a lot of issues until after deployment, since we don't actually know how things will turn out until the deployment happens. Deploying is a bit like running code you write locally - it feels kinda wrong to ask people to review it without making any deploys, since it's like asking people to review code you haven't run locally...

This is when having some real "staging" scenario could help, IMHO.
I mean, in MY ideal world, you create this PR and deploy it against OUR 2i2c AWS cluster.
Then everyone can review the PR and test the changes for real. We can even invite people from the community to test stuff.
If everything goes OK then, we merge the PR and start the process to deploy the change into production, in this case, the CarbonPlan hub.
You may have the case when the deployment fails or the thing you worked on does not work in the real setting, but those chances are reduced significantly since you already tested on our 2i2c AWS hub. And if it happens, you push back production and debug/iterate on the staging again.
Finally, there could be situations where you really need to develop on the community space, but those should be exceptional cases (when we can not replicate the community state).

[damianavila]

Btw, belated review on the code and LGTM 😉