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

Helm 3: Test Enhancement Proposal #6020

Open
jlegrone opened this issue Jul 12, 2019 · 5 comments

Comments

@jlegrone
Copy link
Member

commented Jul 12, 2019

User-Facing Changes

Features

  1. Support defining tests as Jobs
  2. Enable the management of resources (eg. service accounts, network policy, configmaps) to be made available during test execution
  3. Stream execution logs & events with a flag (eg. --verbose) -- see #3481
  4. Enable control over resource deletion behavior on a per-resource basis (may be overridden globally with a flag, eg. --skip-hook-cleanup)

Breaking Changes

  1. Remove test-failure hook (only 20 usages public on Github)
  2. Remove the existing --cleanup flag
  3. Make resource cleanup the default (on success OR failure), and add a flag to skip cleanup for all resources (--skip-hook-cleanup)

Deprecations

  1. Rename the test-success annotation value to test. Keep supporting test-success but log a warning to the user.

3.x and Beyond

  1. Surface relevant logs and k8s events to users during hook execution or on failure
  2. Support structured output with --output json, eg:
    {
      "logs": [
        {
          "pod": "foo",
          "output": "helloworld"
        }
      ],
      "events": [
        {
          "apiVersion": "v1",
          "involvedObject": {
            "apiVersion": "v1",
            "kind": "Pod",
            "name": "foo"
          },
          "message": "Created container bar",
          "reason": "Created"
        }
      ]
    }

Implementation

  1. Specify tests and supporting resources with the helm.sh/hook: test annotation
  2. Converge the business logic for test execution, log output, deletion policy, etc with existing hook implementations
  3. Add a --skip-hook-cleanup flag to install, upgrade, delete, and test commands which will override per-resource helm.sh/hook-delete-policy behavior
  4. Eventually support log streaming and printing of related events on failure for all Helm hooks

Other Considerations

Some of the desired features for helm test (1 and 2) are already supported by the hook system, while the others would likely be welcome enhancements to existing hooks.

Continuing to share the hooks annotation-based API with test is advantageous in that users have only one API to learn, and if desired some tests may also be executed as hooks. The latter is a use case for charts that define "smoke" tests which can do things like prevent upgrades when the target release is in a bad state.

None of the enhancements outlined in this issue are likely to be blocked by continuing to use the hooks API, however in the future there is a possibility that the hooks API will not be sufficient for some unforeseen testing use cases. We have the opportunity to make a breaking change in Helm 3 and move to a separate API for test annotations which could evolve independently of hooks, however this comes at a large cost and may be premature if we lack well defined use cases predicated on the API.

@jeremy-donson

This comment has been minimized.

Copy link

commented Jul 13, 2019

In general, moving testing, dependency mgt, change management, etc
to standard command features will give helm HUGE steps forward.

I would gladly discuss any and all,
and this bears broad discussion across languages and services.

@michelleN

This comment has been minimized.

Copy link
Member

commented Jul 18, 2019

Excellent write up @jlegrone. I think you've come up with an excellent proposal and I appreciate your perspective as an end user. It's a good evolution from what test is doing now and lays a solid foundation for evolving the test experience in the future. I can see where we might also be able to evolve this into support for test suites or even running single tests as has been requested recently.

Some questions:

  1. Would you still support Pods in this scenario or move entirely to supporting Jobs?
  2. How do you handle the scenario where a user wants to leave test pods around to inspect in case of a failure? This has been a big ask in the community.
@bacongobbler

This comment has been minimized.

Copy link
Member

commented Jul 18, 2019

Thanks for the write-up @jlegrone. As discussed in the call, it may make sense to work on the infrastructure to support these use cases so we can avoid delaying a 3.0 release. Laying out the groundwork would be a good first step, then we can start discussing feature additions proposed, such as log streaming support, Job support, etc. in a future 3.x release. If we can get those feature addtions done before 3.0, that's great, but we should make sure that we have a solid foundation to work upon to ensure we can make this work happen without having to break compatibility.

Are you looking to do this work yourself, or are you looking for others to implement the work? Just trying to gauge what needs to happen to get the ball rolling on this. Thanks!

@bacongobbler

This comment has been minimized.

Copy link
Member

commented Jul 18, 2019

I also want to make sure that we have a story for users migrating from Helm 2 to Helm 3. What's changed, why we're changing it, how to migrate etc.

@jlegrone

This comment has been minimized.

Copy link
Member Author

commented Jul 18, 2019

@michelleN

Would you still support Pods in this scenario or move entirely to supporting Jobs?

We'll support whatever is supported by hooks in helm 3. I wouldn't mind switching entirely to jobs but that would be a pretty major change so it's probably best to continue supporting both.

How do you handle the scenario where a user wants to leave test pods around to inspect in case of a failure?

This should be possible with the helm.sh/hook-delete-policy annotation. Eg. always leave resources around for inspection:

  annotations:
    helm.sh/hook-delete-policy: before-hook-creation

Or only skip cleanup on failure:

  annotations:
    helm.sh/hook-delete-policy: hook-succeeded

The proposed --skip-hook-cleanup flag can be added after 3.0, but would also help address debugging use cases when working with charts that may not directly expose those annotations.

@bacongobbler

Are you looking to do this work yourself, or are you looking for others to implement the work? Just trying to gauge what needs to happen to get the ball rolling on this.

Right now the plan is to work with @michelleN on implementing required changes for 3.0, and we can break out new features for 3.1+ releases into independent github issues. I'm definitely happy to take on some of the development effort!

@bacongobbler bacongobbler added this to the 3.0.0-beta.1 milestone Jul 18, 2019

@jlegrone jlegrone referenced this issue Jul 20, 2019
2 of 3 tasks complete

@bacongobbler bacongobbler removed this from the 3.0.0-beta.1 milestone Aug 12, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.