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

MBS-11834 [Instrumentation plugin] add instrumentation.environment k8s config #1265

Merged
merged 1 commit into from
Nov 1, 2021

Conversation

dsvoronin
Copy link
Contributor

instead of implicit gradle parameters
"default" config added for backward compatibility

@dsvoronin dsvoronin requested a review from sboishtyan October 29, 2021 21:47
@dsvoronin dsvoronin changed the title MBS-11834 [Instrumentation plugin ]add instrumentation.environment k8s config MBS-11834 [Instrumentation plugin] add instrumentation.environment k8s config Oct 29, 2021
@dsvoronin dsvoronin force-pushed the MBS-11834-k8s-config branch 3 times, most recently from 5d1200c to 6c08636 Compare November 1, 2021 14:35
instead of implicit gradle parameters
"default" config added for backward compatibility
@dsvoronin dsvoronin force-pushed the MBS-11834-k8s-config branch from 6c08636 to ac72363 Compare November 1, 2021 17:03
@dsvoronin dsvoronin merged commit 950aaf0 into develop Nov 1, 2021
@dsvoronin dsvoronin deleted the MBS-11834-k8s-config branch November 1, 2021 18:29
|}
|
| environments {
| register<com.avito.instrumentation.configuration.KubernetesViaContext>("k8sContext") {
Copy link
Contributor

Choose a reason for hiding this comment

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

It will be simpler for users If they can write

environments {
    kubernetesViaContext("name") { }
    ... etc
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

looks good, yes

/**
* todo remove (registered for backward compatibility)
*/
private fun registerDefaultEnvironment(
Copy link
Contributor

Choose a reason for hiding this comment

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

When do you plan to remove default env?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right after CISteps removal, because almost every build uses instrumentation tasks, which uses default environment for now; so in a couple of days

when (environment) {
is KubernetesViaCredentials ->
task.kubernetesCredentials.set(
KubernetesCredentials.Service(
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like we want to reuse those models

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure that we need to reuse models for gradle public API,
if it will be covered by proposed dsl abstraction: #1265 (comment) then yes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants