Skip to content

feat: Add configuration flag to make canary releases optional in LMOS… #62

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

Open
bharatnpti wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: Add configuration flag to make canary releases optional in LMOS… #62

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/main/helm/templates/deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ spec:
- name: SPRING_PROFILES_ACTIVE
value: "logformat-json"
{{- end }}
- name: LMOS_OPERATOR_CANARY_RELEASE_ENABLED
value: "{{ .Values.canaryReleaseEnabled }}
ports:
- name: http
containerPort: {{ .Values.service.port }}
Expand Down
2 changes: 2 additions & 0 deletions src/main/helm/templates/virtualservice.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
{{- if .Values.canaryReleaseEnabled }}
apiVersion: networking.istio.io/v1
kind: VirtualService
metadata:
Expand Down Expand Up @@ -33,3 +34,4 @@ spec:
x-namespace: {{ $namespace | quote }}
x-subset: "canary"
{{- end }}
{{- end }}
2 changes: 2 additions & 0 deletions src/main/helm/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,3 +135,5 @@ customResources:
# The namespaces from where clients are calling the lmos operator
clientNamespaces:
- default

canaryReleaseEnabled: true
5 changes: 3 additions & 2 deletions src/main/kotlin/org/eclipse/lmos/operator/reconciler/generator/RoutingChannelGenerator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import org.eclipse.lmos.operator.resources.ChannelRoutingCapability
import org.eclipse.lmos.operator.resources.ChannelRoutingResource
import org.eclipse.lmos.operator.resources.ChannelRoutingSpec
import org.eclipse.lmos.operator.resources.Labels
import org.eclipse.lmos.operator.util.SubsetProvider
import org.slf4j.Logger
import org.slf4j.LoggerFactory

Expand Down Expand Up @@ -49,7 +50,7 @@ fun createChannelRoutingResource(channelResource: ChannelResource): ChannelRouti
val metadata = channelResource.metadata
val channelResourceName = metadata.name
val labels = metadata.labels
LOG.debug("Channel labels:{}", labels)
LOG.info("Channel:{}, Channel labels:{}", channelResource.metadata.name, labels)
val channel = labels[Labels.CHANNEL]
val tenant = labels[Labels.TENANT]
val version = labels[Labels.VERSION]
Expand All @@ -63,7 +64,7 @@ fun createChannelRoutingResource(channelResource: ChannelResource): ChannelRouti
Labels.CHANNEL to channel,
Labels.TENANT to tenant,
Labels.VERSION to version,
Labels.SUBSET to subset,
Labels.SUBSET to SubsetProvider.getSubset(subset),
)
channelRoutingResource.metadata = channelRoutingResourceMetadata
channelRoutingResource.spec = ChannelRoutingSpec()
Expand Down
6 changes: 3 additions & 3 deletions src/main/kotlin/org/eclipse/lmos/operator/server/routing/CustomResourcesController.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,14 @@ private const val KUBERNETES_DEFAULT_NAMESPACE = "default"
class CustomResourcesController(private val customResourcesService: CustomResourcesService) {
@GetMapping("/channels")
fun getChannels(
@RequestHeader(name = X_SUBSET_HEADER) subsetHeader: String,
@RequestHeader(name = X_SUBSET_HEADER, required = false) subsetHeader: String,
Copy link
Contributor

@hsudbrock hsudbrock Feb 19, 2025

Choose a reason for hiding this comment

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

If we set required to false, should we also change the type of subsetHeaderto String? to make it nullable? Or will spring somehow take care that we don't get a NPE here?

@RequestHeader(name = X_NAMESPACE_HEADER, required = false) namespaceHeader: String = KUBERNETES_DEFAULT_NAMESPACE,
@PathVariable tenant: String,
) = customResourcesService.getChannels(tenant, subsetHeader, namespaceHeader)

@GetMapping("/channels/{channel}", produces = [MediaType.APPLICATION_JSON_VALUE])
fun getChannel(
@RequestHeader(name = X_SUBSET_HEADER) subsetHeader: String,
@RequestHeader(name = X_SUBSET_HEADER, required = false) subsetHeader: String,
@RequestHeader(name = X_NAMESPACE_HEADER, required = false) namespaceHeader: String = KUBERNETES_DEFAULT_NAMESPACE,
@PathVariable tenant: String,
@PathVariable channel: String,
Expand All @@ -44,7 +44,7 @@ class CustomResourcesController(private val customResourcesService: CustomResour

@GetMapping("/channels/{channel}/routing", produces = [MediaType.APPLICATION_JSON_VALUE])
fun getRouting(
@RequestHeader(name = X_SUBSET_HEADER) subsetHeader: String,
@RequestHeader(name = X_SUBSET_HEADER, required = false) subsetHeader: String,
@RequestHeader(name = X_NAMESPACE_HEADER, required = false) namespaceHeader: String = KUBERNETES_DEFAULT_NAMESPACE,
@PathVariable tenant: String,
@PathVariable channel: String,
Expand Down
7 changes: 4 additions & 3 deletions ...ain/kotlin/org/eclipse/lmos/operator/server/routing/impl/DefaultCustomResourcesService.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import io.fabric8.kubernetes.client.KubernetesClient
import org.eclipse.lmos.operator.resources.ChannelResource
import org.eclipse.lmos.operator.resources.ChannelRoutingResource
import org.eclipse.lmos.operator.server.routing.CustomResourcesService
import org.eclipse.lmos.operator.util.SubsetProvider
import org.springframework.stereotype.Component
import java.util.function.Consumer

Expand All @@ -25,8 +26,8 @@ class DefaultCustomResourcesService(private val client: KubernetesClient) : Cust
mapOf(
"tenant" to tenant,
"channel" to channel,
"subset" to subset,
)
labelSelectors.plus("subset" to SubsetProvider.getSubset(subset))

val channelRoutingResources =
client
Expand All @@ -52,8 +53,8 @@ class DefaultCustomResourcesService(private val client: KubernetesClient) : Cust
val labelSelectors =
mapOf(
"tenant" to tenant,
"subset" to subset,
)
labelSelectors.plus("subset" to SubsetProvider.getSubset(subset))

val channelResources =
client.resources(
Expand All @@ -77,8 +78,8 @@ class DefaultCustomResourcesService(private val client: KubernetesClient) : Cust
mapOf(
"tenant" to tenant,
"channel" to channel,
"subset" to subset,
)
labelSelectors.plus("subset" to SubsetProvider.getSubset(subset))

val channelResources =
client.resources(
Expand Down
17 changes: 17 additions & 0 deletions src/main/kotlin/org/eclipse/lmos/operator/util/SubsetProvider.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package org.eclipse.lmos.operator.util

object SubsetProvider {

private val canaryReleaseEnabled: Boolean by lazy {
val envValue = System.getenv("LMOS_OPERATOR_CANARY_RELEASE_ENABLED")
Copy link
Contributor

@hsudbrock hsudbrock Feb 19, 2025

Choose a reason for hiding this comment

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

As the lmos-operator is built using spring boot, I asked myself whether we could (and should) use spring boot's configuration to inject the "canaryReleaseEnabled" flag (instead of bypassing spring boot and reading from system environment directly).

I see that it is currently not immediately possible, because the SubsetProvider is used by the ChannelDependentResource (via the RoutingChannelGenerator), and the ChannelDependentResource is currently not managed by spring. But if we marked it as a @Component, we could inject the flag via spring boot?

envValue?.toBoolean() ?: true
}

fun getSubset(subsetValue: String?): String {
return if (canaryReleaseEnabled) {
subsetValue ?: throw IllegalArgumentException("Subset value is required when canary release is enabled")
} else {
subsetValue ?: "stable"
}
}
}
2 changes: 1 addition & 1 deletion src/main/resources/application.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,4 +55,4 @@ management:
prometheus:
metrics:
export:
enabled: true
enabled: true