-
Notifications
You must be signed in to change notification settings - Fork 6
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
Port 12682 support new provisioning on k8s-exporter #94
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make sure this configuration is also available when reading configuration from the ConfigMap
pkg/defaults/init.go
Outdated
if applicationConfig.CreatePortResourcesOrigin == port.CreatePortResourcesOriginPort { | ||
klog.Info("Resources origin is set to be Port, verifying integration is supported") | ||
featureFlags, err := org_details.GetOrganizationFeatureFlags(portClient) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
hasProvisionedDefaultsFlag := false | ||
for _, flag := range featureFlags { | ||
if flag == port.OrgUseProvisionedDefaultsFeatureFlag { | ||
hasProvisionedDefaultsFlag = true | ||
break | ||
} | ||
} | ||
|
||
if !hasProvisionedDefaultsFlag { | ||
klog.Info("Port origin for Integration is not supported, changing resources origin to use K8S") | ||
applicationConfig.CreatePortResourcesOrigin = port.CreatePortResourcesOriginK8S | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you put that in a different function, it could be something generic like isFeatureFalgSupported
pkg/defaults/init.go
Outdated
@@ -38,7 +61,8 @@ func InitIntegration(portClient *cli.PortClient, applicationConfig *port.Config) | |||
} | |||
|
|||
klog.Warningf("Could not get integration with state key %s, error: %s", applicationConfig.StateKey, err.Error()) | |||
if err := integration.CreateIntegration(portClient, applicationConfig.StateKey, applicationConfig.EventListenerType, defaultIntegrationConfig); err != nil { | |||
_, err := integration.CreateIntegration(portClient, applicationConfig.StateKey, applicationConfig.EventListenerType, defaultIntegrationConfig, applicationConfig.CreatePortResourcesOrigin == port.CreatePortResourcesOriginPort) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you could use a helper variable here instead of evaluating twice
shouldCreateResourcesUsingPort = applicationConfig.CreatePortResourcesOrigin == port.CreatePortResourcesOriginPort
8ffeacc
to
a91b2de
Compare
pkg/defaults/init.go
Outdated
@@ -47,7 +80,7 @@ func InitIntegration(portClient *cli.PortClient, applicationConfig *port.Config) | |||
EventListener: getEventListenerConfig(applicationConfig.EventListenerType), | |||
} | |||
|
|||
if existingIntegration.Config == nil || applicationConfig.OverwriteConfigurationOnRestart { | |||
if (existingIntegration.Config == nil || applicationConfig.OverwriteConfigurationOnRestart) && !(applicationConfig.CreatePortResourcesOrigin == port.CreatePortResourcesOriginPort) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
REF
@@ -47,7 +80,7 @@ func InitIntegration(portClient *cli.PortClient, applicationConfig *port.Config) | |||
EventListener: getEventListenerConfig(applicationConfig.EventListenerType), | |||
} | |||
|
|||
if existingIntegration.Config == nil || applicationConfig.OverwriteConfigurationOnRestart { | |||
if (existingIntegration.Config == nil && !(applicationConfig.CreatePortResourcesOrigin == port.CreatePortResourcesOriginPort)) || applicationConfig.OverwriteConfigurationOnRestart { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is that correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Description
What - Added support to create resources on port by default
Why - Support onboarding experience
How - Decide which logic to execute by port FF
Type of change
Please leave one option from the following and delete the rest: