Remove two-deployment mode

[dmitryax]

As agreed in #91, we need remove the two-deployment mode and simplify the configuration.

Instead of agentCollector and standaloneCollector, we should have another config option mode which can have one of the following values: daemonset or deployment. All the nested configuration options from agentCollector and standaloneCollector should be moved to the root level. Some of them can be no-op based on the selected mode.

Ideally we need to provide backward compatibility with existing interface for now. It means that we translate agentCollector and standaloneCollector configs into the new config and show deprecation warning if they are used.

If user has both modes enabled (agentCollector.enabled=true and standaloneCollector.enabled=true) the upgrade should fail. This can be achieved by updating schema.values.json. For this use case we need to provide a detailed upgrade guideline on how to replace it with two separate helm installations.

[TylerHelmuth]

I will take a look at this.

[TylerHelmuth]

@dmitryax If we are willing to break backwards compatibility for both modes enabled, should we just rip the bandaid off and make agentCollector and standaloneCollector break the install as well? Since we aren't giving the both-mode-enabled customers a chance to convert before failure should we take that stance across the board and force anyone using this version of the charts to update their values.yaml? If we do this now it will simplify the charts immediately instead of having to maintain merging in agentCollector and standaloneCollector

[dmitryax]

I think majority of the use cases is enabled one mode or another. So I would say we should not break their upgrade

[TylerHelmuth]

The situation I dislike is that since agentCollector.enabled is true by default, there will be customers the have configurations like:

agentCollector:
  resources:
    limits:
      cpu: 2
      memory: 4Gi

which relies on agentCollector.enabled to be true by default. To continue to support these customers after this refactor we need to make sure their config continues to work, which means we have to keep agentCollector.enabled true by default. This results in strange values.yamls for users who want to use the new mode: daemonset and root-level config options. To use the new mode and config options your values.yaml would look like

// daemonset is the default so this technically isn't needed
mode: daemonset
agentCollector:
  enabled: false
resources:
  limits:
    cpu: 2
    memory: 4Gi

I don't love that the default values.yaml would still be using the agentCollector configs and any new user would have to reference the old config setting even though we are trying to deprecate it :(

[dmitryax]

Sorry I don't think I understand your concern. Are you saying that users have to set agentCollector.enabled: true in their values.yaml even if they have mode: daemonset?

[TylerHelmuth]

If users want to use the mode: daemonset, and the other new configurations at the root level such as resources or podAnnotations, they will have to explicitly list agentCollector.enabled: false in their values.yaml since we have to keep agentCollector.enabled: true in the chart's default values.yaml to maintain backwards compatibility.

I think the only way to deal with that potentially confusing agentCollector.enabled: false entry in values.yamls that migrated to the new mode capability is to break backwards compatibility. We can do that now or we can wait and give customers time to switch. The annoying thing is that as part of that switch they will have to put agentCollector.enabled: false in their values.yaml, and then at a future point when we truly remove agentCollector they will have to go into their values.yaml again and remove agentCollector.enabled: false.

[dmitryax]

What if we don't set mode by default and use it to distinguish whether user updated their config or not? The following expression can help with that eq (toString .Values.mode) "<nil>".

• If mode is unset, we use the old format and throw a deprecation warning.
• If mode is set, we completely ignore agentCollector and standaloneCollector configurations.

Later we will just drop agentCollector and standaloneCollector.

[TylerHelmuth]

Yes I like that a lot. I will get the PR updated with that strategy, see how it comes out.