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

[E2E Scenario tests] Add initial set of e2e tests for k8s upgrade command for non-ha mode. #633

Closed
mukundansundar opened this issue Mar 18, 2021 · 4 comments · Fixed by #665
Assignees
Milestone

Comments

@mukundansundar
Copy link
Collaborator

Describe the proposal

[E2E Scenario tests] Add e2e tests for k8s upgrade command

Handle multiple version combinations of upgrade if possible. (Covering the supported versions of Dapr)

Release Note

RELEASE NOTE:

@mukundansundar
Copy link
Collaborator Author

mukundansundar commented Apr 5, 2021

The current draft PR #665 checks that the on upgrade the new crds , new cluster role bindings and new cluster roles exist.
Also that PR is for non-ha mode and MTLS is off in that mode (can be changed).

Additionally without applying the components again, check that already existing components continue to exist after the upgrade.
The question now is should the following scenario also be tested here, considering this is an e2e for the dapr upgrade command in CLI?

  1. should an app be deployed on install and after upgrade redeployed and verify it runs as expected?
  2. should additional resources be tested? (Both for upgrade and on install)
  • roles, role bindings
  • secrets in the dapr namespace related to dapr eg: operator, trust bundle etc.
  • mutating webhook configurer exists
  • should CRD versions be tested onInstall and onUpgrade?
  • is there a deterministic way to say that on upgrade we do not expect the CRDs to be upgraded for this version and if so should it also be added into the check?
  • should the daprsystem configuration be verified as unchanged?
  • what are the other resources that are affected by the upgrade command? Can we also make a list of resources that should not be affected by it so that it can also be verified?
  1. should MTLS enabled mode be verified? I believe Yes
  2. Also that PR checks for MTLS expiry command works and the MTLS export works. Should a check be made for upgrade that checks that the certs are the same on upgrade (if it existed before)?
  3. should HA mode be tested? I believe Yes for this too
  4. should the --set flag also be tested?

My thoughts are for 1, it should not be done here and be done as part of test-infra updates if possible. (But then how will helm upgrade command be tested? since the test-infra is using the helm upgrade process now)

@yaron2 @pkedy @artursouza any thoughts on this?

@artursouza
Copy link
Member

  • I think item 1 should be a valid scenario simply because cli "owns" the upgrade logic.
  • Some of the validations in 2 should be validated but I would avoiding being too specific otherwise, the test will be flaky to small changes to the implementation. For example, I would not check for "mutating webhook configurer exists" instead I would test that the feature that depends on it works - there is a difference.
  • yes to item 3.
  • yes to item 4.
  • yes to item 5.
  • yes to item 6.

Like I said, cli "owns" upgrade and install logic so it should be tested.

LGTM.

@mukundansundar
Copy link
Collaborator Author

  • I think item 1 should be a valid scenario simply because cli "owns" the upgrade logic.
  • Some of the validations in 2 should be validated but I would avoiding being too specific otherwise, the test will be flaky to small changes to the implementation. For example, I would not check for "mutating webhook configurer exists" instead I would test that the feature that depends on it works - there is a difference.
  • yes to item 3.
  • yes to item 4.
  • yes to item 5.
  • yes to item 6.

Like I said, cli "owns" upgrade and install logic so it should be tested.

LGTM.

I will create separate issues for all of these items. I do not believe it should be this issue it self. @artursouza
Will modify this issue for non-ha mode with initial upgrade testing.

I think the mutating webhook configurer is used for addmission control ... as long as the sidecar starts up and the app "works" as expected, i think that should suffice.

@mukundansundar mukundansundar changed the title [E2E Scenario tests] Add e2e tests for k8s upgrade command [E2E Scenario tests] Add e2e tests for k8s upgrade command for non-ha mode. Apr 7, 2021
@mukundansundar
Copy link
Collaborator Author

Added separate issues #674 #675 #676

@mukundansundar mukundansundar changed the title [E2E Scenario tests] Add e2e tests for k8s upgrade command for non-ha mode. [E2E Scenario tests] Add initial set of e2e tests for k8s upgrade command for non-ha mode. Apr 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants