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

*: support "AWS_K8S_TESTER_EKS_SKIP_DELETE_CLUSTER_AND_NODES" #128

Merged
merged 3 commits into from
Jul 16, 2020

Conversation

gyuho
Copy link
Contributor

@gyuho gyuho commented Jul 15, 2020

Partially address #123.

Short-term solution without breaking changes.

@@ -9,9 +9,20 @@

See [code changes](https://github.com/aws/aws-k8s-tester/compare/v1.4.5...v1.4.6).

### `eksconfig`

- Add [`SkipDeleteClusterAndNodes` to skip EKS cluster and nodes deletion](https://github.com/aws/aws-k8s-tester/commit/565635179860b4832b64cbe3b39fdbe1c12b1ae1).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider calling this "Attach to cluster"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of skipping, can we idempotently check that it already exists and just succeed?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking even higher level, how much effort would it be to make everything within AKT idempotent, so that we could modify the yaml config and it would simply execute the deltas?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean skipping creation? Creation is already done here

func (ts *tester) createEKS() (err error) {
fmt.Printf(ts.cfg.EKSConfig.Colorize("\n\n[yellow]*********************************\n"))
fmt.Printf(ts.cfg.EKSConfig.Colorize("[light_green]createEKS [default](%q)\n"), ts.cfg.EKSConfig.ConfigPath)
if ts.cfg.EKSConfig.Status.ClusterCFNStackID != "" ||
ts.cfg.EKSConfig.Status.ClusterARN != "" ||
ts.cfg.EKSConfig.Status.ClusterAPIServerEndpoint != "" ||
ts.cfg.EKSConfig.Status.ClusterCA != "" ||
ts.cfg.EKSConfig.Status.ClusterCADecoded != "" {
ts.cfg.Logger.Info("non-empty cluster given; no need to create a new one", zap.String("status", ts.cfg.EKSConfig.Status.ClusterStatusCurrent))
return nil
}
if ts.cfg.EKSConfig.Status.Up {
ts.cfg.Logger.Info("cluster is up; no need to create cluster")
return nil
}

which means you can run create cluster command multiple times and it will automatically skip if already created.

- If true, delete operation keeps all resources created before cluster (e.g. IAM role, VPC, CMK, etc.).
- Set via `AWS_K8S_TESTER_EKS_SKIP_DELETE_CLUSTER_AND_NODES=true`.
- Default `AWS_K8S_TESTER_EKS_SKIP_DELETE_CLUSTER_AND_NODES=false`.
- `aws-k8s-tester eks create cluster --path` must be passed a valid configuration file with existing cluster information in order to use the existing cluster. Create cluster once, and delete all add-ons, and keep the cluster related fields in the YAML with all other fields removed.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with this path to yaml eventually be our custom resource yaml config?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

@gyuho gyuho force-pushed the skip-delete branch 2 times, most recently from 848953f to 17f720f Compare July 15, 2020 20:30
@gyuho gyuho merged commit 5a7fae4 into aws:master Jul 16, 2020
@gyuho gyuho deleted the skip-delete branch July 16, 2020 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants