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.

Sign up for GitHub

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

Jump to bottom

WIP Ensure ActiveGate is Enabled for Extensions #3361

Closed
wants to merge 2 commits into from
Closed

WIP Ensure ActiveGate is Enabled for Extensions #3361

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
903cd05
enables ExtensionsController module on ActiveGate
aorcholski Jul 8, 2024
0b753a4
ConvertFrom/ConvertTo approach
aorcholski Jul 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 16 additions & 10 deletions cmd/troubleshoot/image.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ import (
"context"
"net/http"

dynatracev1beta2 "github.com/Dynatrace/dynatrace-operator/pkg/api/v1beta2/dynakube"
"github.com/Dynatrace/dynatrace-operator/pkg/api/v1beta2/dynakube"
dynakubev1beta3 "github.com/Dynatrace/dynatrace-operator/pkg/api/v1beta3/dynakube"
Comment on lines +7 to +8
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO, if you use 2 versions, use 2 aliases

Copy link
Contributor

@0sewa0 0sewa0 Jul 9, 2024 •
edited

Choose a reason for hiding this comment

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

I read through more of the code, and it always comes back up.
And I started to ask my self: "What is the benefit of handling 2 versions in a package?"

It is just confusing, and will only cause more work, because when we move away from the older version, all of these packages need to be changed.
There is no information loss when converting between the versions, so what the old-version had the new-version also has. So doing this "use double the version, and sometimes pass in 2 dynakubes" are just asking for mistakes caused by confusion.

IMO The biggest flaw with this whole approach (converting at "random" places) is that we like to pass the DynaKube via reference, and we expect the Status part to be mutated as we pass it around, but the moment you introduce conversions in places, and not handle the mutation carefully, some data may be lost

How I would do it is:

  • Only 1 version is allowed in a package (conversion and main dk-controller is exempt, maybe troubleshoot and istio aswell)
  • If a package uses the new-version in any way, the input for that package should be the new-version
    • The caller of the package will do the convertTo and From, so it will own the reference that is passed it, so no changes to the Status should be lost.

Copy link
Contributor

Choose a reason for hiding this comment

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

While I agree with you, I think we should focus on unblocking this PR (because Werner is on vacation, many stories depends on this and it's getting stucked) and then start switching to beta3 everywhere. We will have to do it anyways with the oncoming stories so we can do it in small increments, or maybe do it all at once after this is merged?

I'm just trying to find the most painless way of moving forward with this.

"github.com/Dynatrace/dynatrace-operator/pkg/arch"
"github.com/Dynatrace/dynatrace-operator/pkg/logd"
"github.com/google/go-containerregistry/pkg/authn"
Expand All @@ -30,25 +31,30 @@ type Auths struct {

type ImagePullFunc func(image string) error

func verifyAllImagesAvailable(ctx context.Context, baseLog logd.Logger, keychain authn.Keychain, transport *http.Transport, dynakube *dynatracev1beta2.DynaKube) error {
func verifyAllImagesAvailable(ctx context.Context, baseLog logd.Logger, keychain authn.Keychain, transport *http.Transport, dk *dynakube.DynaKube) error {
log := baseLog.WithName("imagepull")

imagePullFunc := CreateImagePullFunc(ctx, keychain, transport)

if dynakube.NeedsOneAgent() {
verifyImageIsAvailable(log, imagePullFunc, dynakube, componentOneAgent, false)
verifyImageIsAvailable(log, imagePullFunc, dynakube, componentCodeModules, true)
if dk.NeedsOneAgent() {
verifyImageIsAvailable(log, imagePullFunc, dk, componentOneAgent, false)
verifyImageIsAvailable(log, imagePullFunc, dk, componentCodeModules, true)
}

if dynakube.NeedsActiveGate() {
verifyImageIsAvailable(log, imagePullFunc, dynakube, componentActiveGate, false)
dynakubeV1beta3 := &dynakubev1beta3.DynaKube{}
if err := dynakubeV1beta3.ConvertFrom(dk); err != nil {
return err
}

if dynakubeV1beta3.NeedsActiveGate() {
verifyImageIsAvailable(log, imagePullFunc, dk, componentActiveGate, false)
}

return nil
}

func verifyImageIsAvailable(log logd.Logger, pullImage ImagePullFunc, dynakube *dynatracev1beta2.DynaKube, comp component, proxyWarning bool) {
image, isCustomImage := comp.getImage(dynakube)
func verifyImageIsAvailable(log logd.Logger, pullImage ImagePullFunc, dk *dynakube.DynaKube, comp component, proxyWarning bool) {
image, isCustomImage := comp.getImage(dk)
if comp.SkipImageCheck(image) {
logErrorf(log, "Unknown %s image", comp.String())

Expand All @@ -64,7 +70,7 @@ func verifyImageIsAvailable(log logd.Logger, pullImage ImagePullFunc, dynakube *
return
}

if dynakube.HasProxy() && proxyWarning {
if dk.HasProxy() && proxyWarning {
logWarningf(log, "Proxy setting in Dynakube is ignored for %s image due to technical limitations.", componentName)
}

Expand Down
1 change: 1 addition & 0 deletions pkg/api/v1beta3/dynakube/convert_from.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ func (dst *DynaKube) fromActiveGateSpec(src *v1beta2.DynaKube) {
dst.Spec.ActiveGate.Resources = src.Spec.ActiveGate.Resources
dst.Spec.ActiveGate.Replicas = src.Spec.ActiveGate.Replicas

dst.Spec.ActiveGate.Capabilities = []CapabilityDisplayName{}
for _, capability := range src.Spec.ActiveGate.Capabilities {
dst.Spec.ActiveGate.Capabilities = append(dst.Spec.ActiveGate.Capabilities, CapabilityDisplayName(capability))
}
Expand Down
5 changes: 3 additions & 2 deletions pkg/api/v1beta3/dynakube/convert_to.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,12 @@ func (src *DynaKube) ConvertTo(dstRaw conversion.Hub) error {
}

func (src *DynaKube) toBase(dst *v1beta2.DynaKube) {
dst.ObjectMeta = *src.ObjectMeta.DeepCopy() // DeepCopy mainly relevant for testing

if dst.Annotations == nil {
dst.Annotations = map[string]string{}
}

dst.ObjectMeta = *src.ObjectMeta.DeepCopy() // DeepCopy mainly relevant for testing

dst.Spec.APIURL = src.Spec.APIURL
dst.Spec.Tokens = src.Spec.Tokens
dst.Spec.CustomPullSecret = src.Spec.CustomPullSecret
Expand Down Expand Up @@ -79,6 +79,7 @@ func (src *DynaKube) toActiveGateSpec(dst *v1beta2.DynaKube) {
dst.Spec.ActiveGate.Resources = src.Spec.ActiveGate.Resources
dst.Spec.ActiveGate.Replicas = src.Spec.ActiveGate.Replicas

dst.Spec.ActiveGate.Capabilities = []v1beta2.CapabilityDisplayName{}
for _, capability := range src.Spec.ActiveGate.Capabilities {
dst.Spec.ActiveGate.Capabilities = append(dst.Spec.ActiveGate.Capabilities, v1beta2.CapabilityDisplayName(capability))
}
Expand Down
15 changes: 8 additions & 7 deletions pkg/api/v1beta3/dynakube/properties.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,11 @@ func (dk *DynaKube) OneAgentDaemonsetName() string {
}

func (dk *DynaKube) ActiveGateMode() bool {
return len(dk.Spec.ActiveGate.Capabilities) > 0
return len(dk.Spec.ActiveGate.Capabilities) > 0 || dk.HasExtensionsEnabled()
}

func (dk *DynaKube) HasExtensionsEnabled() bool {
return dk.Spec.Extensions.Prometheus.Enabled
}

func (dk *DynaKube) IsActiveGateMode(mode CapabilityDisplayName) bool {
Expand Down Expand Up @@ -143,14 +147,11 @@ func (dk *DynaKube) IsMetricsIngestActiveGateEnabled() bool {
return dk.IsActiveGateMode(MetricsIngestCapability.DisplayName)
}

func (dk *DynaKube) NeedsActiveGateServicePorts() bool {
func (dk *DynaKube) NeedsActiveGateService() bool {
return dk.IsRoutingActiveGateEnabled() ||
dk.IsApiActiveGateEnabled() ||
dk.IsMetricsIngestActiveGateEnabled()
}

func (dk *DynaKube) NeedsActiveGateService() bool {
return dk.NeedsActiveGateServicePorts()
dk.IsMetricsIngestActiveGateEnabled() ||
dk.HasExtensionsEnabled()
}

func (dk *DynaKube) HasActiveGateCaCert() bool {
Expand Down
22 changes: 11 additions & 11 deletions pkg/controllers/dynakube/activegate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,35 +3,35 @@ package dynakube
import (
"context"

"github.com/Dynatrace/dynatrace-operator/pkg/api/v1beta2/dynakube"
"github.com/Dynatrace/dynatrace-operator/pkg/api/v1beta3/dynakube"
"github.com/Dynatrace/dynatrace-operator/pkg/clients/dynatrace"
"github.com/Dynatrace/dynatrace-operator/pkg/controllers/dynakube/istio"
"github.com/pkg/errors"
)

func (controller *Controller) reconcileActiveGate(ctx context.Context, dynakube *dynakube.DynaKube, dtc dynatrace.Client, istioClient *istio.Client) error {
reconciler := controller.activeGateReconcilerBuilder(controller.client, controller.apiReader, dynakube, dtc, istioClient, controller.tokens)
func (controller *Controller) reconcileActiveGate(ctx context.Context, dk *dynakube.DynaKube, dtc dynatrace.Client, istioClient *istio.Client) error {
reconciler := controller.activeGateReconcilerBuilder(controller.client, controller.apiReader, dk, dtc, istioClient, controller.tokens)
err := reconciler.Reconcile(ctx)

if err != nil {
return errors.WithMessage(err, "failed to reconcile ActiveGate")
}

controller.setupAutomaticApiMonitoring(ctx, dtc, dynakube)
controller.setupAutomaticApiMonitoring(ctx, dtc, dk)

return nil
}

func (controller *Controller) setupAutomaticApiMonitoring(ctx context.Context, dtc dynatrace.Client, dynakube *dynakube.DynaKube) {
if dynakube.Status.KubeSystemUUID != "" &&
dynakube.FeatureAutomaticKubernetesApiMonitoring() &&
dynakube.IsKubernetesMonitoringActiveGateEnabled() {
clusterLabel := dynakube.FeatureAutomaticKubernetesApiMonitoringClusterName()
func (controller *Controller) setupAutomaticApiMonitoring(ctx context.Context, dtc dynatrace.Client, dk *dynakube.DynaKube) {
if dk.Status.KubeSystemUUID != "" &&
dk.FeatureAutomaticKubernetesApiMonitoring() &&
dk.IsKubernetesMonitoringActiveGateEnabled() {
clusterLabel := dk.FeatureAutomaticKubernetesApiMonitoringClusterName()
if clusterLabel == "" {
clusterLabel = dynakube.Name
clusterLabel = dk.Name
}

err := controller.apiMonitoringReconcilerBuilder(dtc, dynakube, clusterLabel, dynakube.Status.KubeSystemUUID).
err := controller.apiMonitoringReconcilerBuilder(dtc, dk, clusterLabel, dk.Status.KubeSystemUUID).
Reconcile(ctx)
if err != nil {
log.Error(err, "could not create setting")
Expand Down
68 changes: 40 additions & 28 deletions pkg/controllers/dynakube/activegate/capability/capability.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,19 @@ import (
"fmt"
"strings"

dynatracev1beta2 "github.com/Dynatrace/dynatrace-operator/pkg/api/v1beta2/dynakube"
"github.com/Dynatrace/dynatrace-operator/pkg/api/v1beta3/dynakube"
"github.com/Dynatrace/dynatrace-operator/pkg/controllers/dynakube/activegate/consts"
"k8s.io/utils/net"
)

type baseFunc func() *capabilityBase

var (
activeGateCapabilities = map[dynatracev1beta2.CapabilityDisplayName]baseFunc{
dynatracev1beta2.KubeMonCapability.DisplayName: kubeMonBase,
dynatracev1beta2.RoutingCapability.DisplayName: routingBase,
dynatracev1beta2.MetricsIngestCapability.DisplayName: metricsIngestBase,
dynatracev1beta2.DynatraceApiCapability.DisplayName: dynatraceApiBase,
activeGateCapabilities = map[dynakube.CapabilityDisplayName]baseFunc{
dynakube.KubeMonCapability.DisplayName: kubeMonBase,
dynakube.RoutingCapability.DisplayName: routingBase,
dynakube.MetricsIngestCapability.DisplayName: metricsIngestBase,
dynakube.DynatraceApiCapability.DisplayName: dynatraceApiBase,
}
)

Expand All @@ -25,11 +25,11 @@ type Capability interface {
ShortName() string
ArgName() string
DisplayName() string
Properties() *dynatracev1beta2.CapabilityProperties
Properties() *dynakube.CapabilityProperties
}

type capabilityBase struct {
properties *dynatracev1beta2.CapabilityProperties
properties *dynakube.CapabilityProperties
shortName string
argName string
displayName string
Expand All @@ -40,7 +40,7 @@ func (capability *capabilityBase) Enabled() bool {
return capability.enabled
}

func (capability *capabilityBase) Properties() *dynatracev1beta2.CapabilityProperties {
func (capability *capabilityBase) Properties() *dynakube.CapabilityProperties {
return capability.properties
}

Expand All @@ -64,30 +64,42 @@ type MultiCapability struct {
capabilityBase
}

func NewMultiCapability(dk *dynatracev1beta2.DynaKube) Capability {
func NewMultiCapability(dk *dynakube.DynaKube) Capability {
mc := MultiCapability{
capabilityBase{
shortName: consts.MultiActiveGateName,
},
}

if dk == nil || !dk.ActiveGateMode() {
return &mc
}

mc.enabled = true
mc.properties = &dk.Spec.ActiveGate.CapabilityProperties
capabilityNames := []string{}
capabilityDisplayNames := make([]string, len(dk.Spec.ActiveGate.Capabilities))

for i, capName := range dk.Spec.ActiveGate.Capabilities {
if len(dk.Spec.ActiveGate.Capabilities) == 0 && dk.HasExtensionsEnabled() {
mc.properties.Replicas = 1
}

capabilityNames := make([]string, 0)

capabilityDisplayNames := make([]string, 0)

for _, capName := range dk.Spec.ActiveGate.Capabilities {
capabilityGenerator, ok := activeGateCapabilities[capName]
if !ok {
continue
}

capGen := capabilityGenerator()
capabilityNames = append(capabilityNames, capGen.argName)
capabilityDisplayNames[i] = capGen.displayName
capabilityDisplayNames = append(capabilityDisplayNames, capGen.displayName)
}

if dk.HasExtensionsEnabled() {
capabilityNames = append(capabilityNames, "extension_controller")
capabilityDisplayNames = append(capabilityDisplayNames, "extension_controller")
}

mc.argName = strings.Join(capabilityNames, ",")
Expand All @@ -98,45 +110,45 @@ func NewMultiCapability(dk *dynatracev1beta2.DynaKube) Capability {

func kubeMonBase() *capabilityBase {
c := capabilityBase{
shortName: dynatracev1beta2.KubeMonCapability.ShortName,
argName: dynatracev1beta2.KubeMonCapability.ArgumentName,
displayName: string(dynatracev1beta2.KubeMonCapability.DisplayName),
shortName: dynakube.KubeMonCapability.ShortName,
argName: dynakube.KubeMonCapability.ArgumentName,
displayName: string(dynakube.KubeMonCapability.DisplayName),
}

return &c
}

func routingBase() *capabilityBase {
c := capabilityBase{
shortName: dynatracev1beta2.RoutingCapability.ShortName,
argName: dynatracev1beta2.RoutingCapability.ArgumentName,
displayName: string(dynatracev1beta2.RoutingCapability.DisplayName),
shortName: dynakube.RoutingCapability.ShortName,
argName: dynakube.RoutingCapability.ArgumentName,
displayName: string(dynakube.RoutingCapability.DisplayName),
}

return &c
}

func metricsIngestBase() *capabilityBase {
c := capabilityBase{
shortName: dynatracev1beta2.MetricsIngestCapability.ShortName,
argName: dynatracev1beta2.MetricsIngestCapability.ArgumentName,
displayName: string(dynatracev1beta2.MetricsIngestCapability.DisplayName),
shortName: dynakube.MetricsIngestCapability.ShortName,
argName: dynakube.MetricsIngestCapability.ArgumentName,
displayName: string(dynakube.MetricsIngestCapability.DisplayName),
}

return &c
}

func dynatraceApiBase() *capabilityBase {
c := capabilityBase{
shortName: dynatracev1beta2.DynatraceApiCapability.ShortName,
argName: dynatracev1beta2.DynatraceApiCapability.ArgumentName,
displayName: string(dynatracev1beta2.DynatraceApiCapability.DisplayName),
shortName: dynakube.DynatraceApiCapability.ShortName,
argName: dynakube.DynatraceApiCapability.ArgumentName,
displayName: string(dynakube.DynatraceApiCapability.DisplayName),
}

return &c
}

func GenerateActiveGateCapabilities(dk *dynatracev1beta2.DynaKube) []Capability {
func GenerateActiveGateCapabilities(dk *dynakube.DynaKube) []Capability {
return []Capability{
NewMultiCapability(dk),
}
Expand All @@ -150,7 +162,7 @@ func BuildDNSEntryPointWithoutEnvVars(dynakubeName, dynakubeNamespace string, ca
return fmt.Sprintf("%s.%s", BuildServiceName(dynakubeName, capability.ShortName()), dynakubeNamespace)
}

func BuildDNSEntryPoint(dk dynatracev1beta2.DynaKube, capability Capability) string {
func BuildDNSEntryPoint(dk dynakube.DynaKube, capability Capability) string {
entries := []string{}

for _, ip := range dk.Status.ActiveGate.ServiceIPs {
Expand Down