Skip to content

Commit

Permalink
refactor(dependency): replace groovy coordinates and code adjustment …
Browse files Browse the repository at this point in the history
…to accomodate upgrade of groovy 4.x

- Replacing the groovy coordinates from `org.codehaus.groovy` to `org.apache.groovy` supported by groovy 4.x and above versions.

- While upgrading groovy 4.0.15:

Encounter below error during test executions of orca-clouddriver module and similar errors in orca-pipelinetemplate module:
```
No such property: disabled for class: com.netflix.spinnaker.orca.clouddriver.pipeline.servergroup.support.TargetServerGroup
groovy.lang.MissingPropertyException: No such property: disabled for class: com.netflix.spinnaker.orca.clouddriver.pipeline.servergroup.support.TargetServerGroup
	at app//com.netflix.spinnaker.orca.clouddriver.tasks.cluster.AbstractClusterWideClouddriverTask.isActive(AbstractClusterWideClouddriverTask.groovy:266)
	at com.netflix.spinnaker.orca.clouddriver.tasks.cluster.AbstractClusterWideClouddriverTask.filterActiveGroups_closure9(AbstractClusterWideClouddriverTask.groovy:187)
	at app//com.netflix.spinnaker.orca.clouddriver.tasks.cluster.AbstractClusterWideClouddriverTask.filterActiveGroups(AbstractClusterWideClouddriverTask.groovy:187)
	at app//com.netflix.spinnaker.orca.clouddriver.tasks.cluster.ScaleDownClusterTask.filterServerGroups(ScaleDownClusterTask.groovy:75)
	at com.netflix.spinnaker.orca.clouddriver.tasks.cluster.ScaleDownClusterTaskSpec.extracts config from context(ScaleDownClusterTaskSpec.groovy:49)
```
To fix this issue, replaced the fields with its getter method.

Encounter below error during compilation of orca-clouddriver module:
```
startup failed:
/orca/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/kato/pipeline/support/SourceResolver.groovy: 118: [Static type checking] - Cannot find matching method ?#compareTo(?). Please check if the declared type is correct and if the method exists.
 @ line 118, column 92.
   ze(a1)<=> instanceSize(a2) ?: created(a2
                                 ^

1 error

> Task :orca-clouddriver:compileGroovy FAILED
```
The root cause seems to be the type mismatch returned by `created` closure. To fix this issue typecasted the return value of `0` from Integer to Long.

Encounter below error during test executions of orca-clouddriver module and similar errors in orca-pipelinetemplate module:
```
No signature of method: com.netflix.spinnaker.orca.kato.tasks.JarDiffsTask.getLog() is applicable for argument types: () values: []
Possible solutions: getAt(java.lang.String), getClass(), notify(), $getLookup(), every(), grep()
groovy.lang.MissingMethodException: No signature of method: com.netflix.spinnaker.orca.kato.tasks.JarDiffsTask.getLog() is applicable for argument types: () values: []
Possible solutions: getAt(java.lang.String), getClass(), notify(), $getLookup(), every(), grep()
	at com.netflix.spinnaker.orca.kato.tasks.JarDiffsTask.getJarList_closure2(JarDiffsTask.groovy:150)
	at app//com.netflix.spinnaker.orca.kato.tasks.JarDiffsTask.getJarList(JarDiffsTask.groovy:143)
	at app//org.spockframework.mock.runtime.ByteBuddyMethodInvoker.respond(ByteBuddyMethodInvoker.java:20)
	at app//org.spockframework.mock.runtime.MockInvocation.callRealMethod(MockInvocation.java:61)
	at app//org.spockframework.mock.CallRealMethodResponse.respond(CallRealMethodResponse.java:30)
	at app//org.spockframework.mock.runtime.MockController.handle(MockController.java:50)
	at app//org.spockframework.mock.runtime.JavaMockInterceptor.intercept(JavaMockInterceptor.java:86)
	at app//org.spockframework.mock.runtime.ByteBuddyInterceptorAdapter.interceptNonAbstract(ByteBuddyInterceptorAdapter.java:35)
	at com.netflix.spinnaker.orca.kato.tasks.JarDiffsTaskSpec.getJarList single server(JarDiffsTaskSpec.groovy:87)
```

```
No signature of method: com.netflix.spinnaker.orca.kato.tasks.quip.TriggerQuipTask.getLog() is applicable for argument types: () values: []
Possible solutions: getAt(java.lang.String), getClass(), notify(), $getLookup(), $getLookup(), every()
groovy.lang.MissingMethodException: No signature of method: com.netflix.spinnaker.orca.kato.tasks.quip.TriggerQuipTask.getLog() is applicable for argument types: () values: []
Possible solutions: getAt(java.lang.String), getClass(), notify(), $getLookup(), $getLookup(), every()
	at com.netflix.spinnaker.orca.kato.tasks.quip.TriggerQuipTask.execute_closure2(TriggerQuipTask.groovy:87)
	at app//groovy.lang.Closure.call(Closure.java:433)
	at app//com.netflix.spinnaker.orca.kato.tasks.quip.TriggerQuipTask.execute(TriggerQuipTask.groovy:71)
	at app//org.spockframework.mock.runtime.ByteBuddyMethodInvoker.respond(ByteBuddyMethodInvoker.java:20)
	at app//org.spockframework.mock.runtime.MockInvocation.callRealMethod(MockInvocation.java:61)
	at app//org.spockframework.mock.CallRealMethodResponse.respond(CallRealMethodResponse.java:30)
	at app//org.spockframework.mock.runtime.MockController.handle(MockController.java:50)
	at app//org.spockframework.mock.runtime.JavaMockInterceptor.intercept(JavaMockInterceptor.java:86)
	at app//org.spockframework.mock.runtime.ByteBuddyInterceptorAdapter.interceptNonAbstract(ByteBuddyInterceptorAdapter.java:35)
	at com.netflix.spinnaker.orca.kato.tasks.quip.TriggerQuipTaskSpec.servers return errors, expect RUNNING(TriggerQuipTaskSpec.groovy:175)
```
To fix this issue, explicitly defined a method to return `log` object inside `find{}` and `each{}` closure. It may be related to this [issue](https://issues.apache.org/jira/browse/GROOVY-8820)

micronaut-projects/micronaut-core#4933 (comment)

Encounter below error during test executions of orca-clouddriver module:
```
Condition not satisfied:

response.context.terminationInstanceIds == expectedTerminations
|        |       |                      |  |
|        |       [i-2, i-3, i-4]        |  [i-4, i-2, i-3]
|        |                              false
|        [terminationInstanceIds:[i-2, i-3, i-4], knownInstanceIds:[i-1, i-2, i-3, i-4], skipRemainingWait:true, waitTime:0]
TaskResult(status=SUCCEEDED, context={terminationInstanceIds=[i-2, i-3, i-4], knownInstanceIds=[i-1, i-2, i-3, i-4], skipRemainingWait=true, waitTime=0}, outputs={})

Condition not satisfied:

response.context.terminationInstanceIds == expectedTerminations
|        |       |                      |  |
|        |       [i-2, i-3, i-4]        |  [i-4, i-2, i-3]
|        |                              false
|        [terminationInstanceIds:[i-2, i-3, i-4], knownInstanceIds:[i-1, i-2, i-3, i-4], skipRemainingWait:true, waitTime:0]
TaskResult(status=SUCCEEDED, context={terminationInstanceIds=[i-2, i-3, i-4], knownInstanceIds=[i-1, i-2, i-3, i-4], skipRemainingWait=true, waitTime=0}, outputs={})

	at com.netflix.spinnaker.orca.kato.tasks.rollingpush.DetermineTerminationCandidatesTaskSpec.should order and filter instances correctly(DetermineTerminationCandidatesTaskSpec.groovy:58)

```
The root cause of this issue is breaking change, in the usage of `intersect()` method, introduced in groovy 4 [here](https://groovy-lang.org/releasenotes/groovy-4.0.html#Groovy4.0-breaking). To fix this issue, refactoring the code.
  • Loading branch information
j-sandy committed Sep 16, 2024
1 parent ff987dc commit 5bfd93a
Show file tree
Hide file tree
Showing 18 changed files with 34 additions and 24 deletions.
2 changes: 1 addition & 1 deletion gradle/groovy.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
apply plugin: "groovy"

dependencies {
implementation("org.codehaus.groovy:groovy")
implementation("org.apache.groovy:groovy")
testImplementation("org.spockframework:spock-core")
testImplementation("cglib:cglib-nodep")
testImplementation("org.objenesis:objenesis")
Expand Down
2 changes: 1 addition & 1 deletion gradle/spock.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ dependencies {
testImplementation("org.spockframework:spock-core")
testImplementation("cglib:cglib-nodep")
testImplementation("org.objenesis:objenesis")
testImplementation("org.codehaus.groovy:groovy")
testImplementation("org.apache.groovy:groovy")
testRuntimeOnly("org.junit.jupiter:junit-jupiter-engine")
}

Expand Down
2 changes: 1 addition & 1 deletion orca-bakery/orca-bakery.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ dependencies {
implementation("com.fasterxml.jackson.core:jackson-databind")
implementation("com.fasterxml.jackson.datatype:jackson-datatype-guava")
implementation("io.spinnaker.fiat:fiat-core:$fiatVersion")
implementation("org.codehaus.groovy:groovy-datetime")
implementation("org.apache.groovy:groovy-datetime")
implementation("io.spinnaker.kork:kork-retrofit")

api("io.spinnaker.kork:kork-web")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ abstract class AbstractClusterWideClouddriverTask implements RetryableTask, Clou
}

static boolean isActive(TargetServerGroup serverGroup) {
return serverGroup.disabled == false || serverGroup.instances.any { it.healthState == HealthState.Up }
return serverGroup.isDisabled() == false || serverGroup.instances.any { it.healthState == HealthState.Up }
}

static class IsActive implements Comparator<TargetServerGroup> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ class SourceResolver {
}

def instanceSize = { ServerGroup asg -> asg.instances?.size() ?: 0 }
def created = { ServerGroup asg -> asg.createdTime ?: 0 }
def created = { ServerGroup asg -> asg.createdTime ?: 0L }
if (stageData.useSourceCapacity) {
regionalAsgs = regionalAsgs.sort { a1, a2 -> instanceSize(a1) <=> instanceSize(a2) ?: created(a2) <=> created(a1) }
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package com.netflix.spinnaker.orca.kato.tasks
import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerRetrofitErrorHandler
import com.netflix.spinnaker.orca.api.pipeline.models.StageExecution
import com.netflix.spinnaker.orca.clouddriver.model.Instance.InstanceInfo
import org.slf4j.Logger
import retrofit.converter.JacksonConverter

import java.util.concurrent.TimeUnit
Expand Down Expand Up @@ -66,6 +67,10 @@ class JarDiffsTask implements DiffTask {

int platformPort = 8077

private Logger getLog() {
return log
}

@Override
public TaskResult execute(StageExecution stage) {
def retriesRemaining = stage.context.jarDiffsRetriesRemaining != null ? stage.context.jarDiffsRetriesRemaining : MAX_RETRIES
Expand Down Expand Up @@ -137,19 +142,19 @@ class JarDiffsTask implements DiffTask {
int numberOfInstancesChecked = 0;
instances.find { key, instanceInfo ->
if (numberOfInstancesChecked++ >= 5) {
log.info("Unable to check jar list after 5 attempts, giving up!")
getLog().info("Unable to check jar list after 5 attempts, giving up!")
return true
}

String hostName = instanceInfo.privateIpAddress ?: instanceInfo.hostName
log.debug("attempting to get a jar list from : ${key} (${hostName}:${platformPort})")
getLog().debug("attempting to get a jar list from : ${key} (${hostName}:${platformPort})")
def instanceService = createInstanceService("http://${hostName}:${platformPort}")
try {
def instanceResponse = instanceService.getJars()
jarMap = objectMapper.readValue(instanceResponse.body.in().text, Map)
return true
} catch(Exception e) {
log.debug("could not get a jar list from : ${key} (${hostName}:${platformPort}) - ${e.message}")
getLog().debug("could not get a jar list from : ${key} (${hostName}:${platformPort}) - ${e.message}")
// swallow it so we can try the next instance
return false
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import com.netflix.spinnaker.orca.api.pipeline.models.StageExecution
import com.netflix.spinnaker.orca.api.pipeline.TaskResult
import com.netflix.spinnaker.orca.clouddriver.InstanceService
import groovy.util.logging.Slf4j
import org.slf4j.Logger
import org.springframework.beans.factory.annotation.Autowired
import org.springframework.stereotype.Component
import retrofit.client.Client
Expand All @@ -45,6 +46,10 @@ class TriggerQuipTask extends AbstractQuipTask implements RetryableTask {
long backoffPeriod = 10000
long timeout = 600000 // 10min

private Logger getLog() {
return log
}

@Override
TaskResult execute(StageExecution stage) {
Map taskIdMap = [:]
Expand Down Expand Up @@ -76,7 +81,7 @@ class TriggerQuipTask extends AbstractQuipTask implements RetryableTask {
taskIdMap.put(instanceHostName, ref.substring(1 + ref.lastIndexOf('/')))
patchedInstanceIds << instanceId
} catch (SpinnakerServerException e) {
log.warn("Error in Quip request: {}", e.message)
getLog().warn("Error in Quip request: {}", e.message)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ class DetermineTerminationCandidatesTask implements Task {
.orElseGet { serverGroupInstances*.instanceId }
def terminationInstancePool = knownInstanceIds
if (stage.context.termination?.instances) {
terminationInstancePool = knownInstanceIds.intersect(stage.context.termination?.instances)
terminationInstancePool = stage.context.termination?.instances.intersect(knownInstanceIds)
if (stage.context.termination.order == 'given') {
terminationInstancePool = terminationInstancePool.sort { stage.context.termination.instances.indexOf(it) }
}
Expand Down
2 changes: 1 addition & 1 deletion orca-core/orca-core.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ dependencies {
implementation("javax.validation:validation-api")
implementation("com.jayway.jsonpath:json-path")
implementation("org.yaml:snakeyaml")
implementation("org.codehaus.groovy:groovy")
implementation("org.apache.groovy:groovy")
implementation("net.javacrumbs.shedlock:shedlock-spring:4.44.0")
implementation("net.javacrumbs.shedlock:shedlock-provider-jdbc-template:4.44.0")

Expand Down
4 changes: 2 additions & 2 deletions orca-front50/orca-front50.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ dependencies {
implementation(project(":orca-core"))
implementation(project(":orca-retrofit"))

api("org.codehaus.groovy:groovy")
api("org.apache.groovy:groovy")

implementation("io.spinnaker.fiat:fiat-api:$fiatVersion")
implementation("io.spinnaker.fiat:fiat-core:$fiatVersion")
Expand All @@ -38,7 +38,7 @@ dependencies {
testImplementation(project(":orca-test-groovy"))
testImplementation(project(":orca-pipelinetemplate"))
testImplementation("com.github.ben-manes.caffeine:guava")
testImplementation("org.codehaus.groovy:groovy-json")
testImplementation("org.apache.groovy:groovy-json")
testRuntimeOnly("net.bytebuddy:byte-buddy")
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ dependencies {
implementation(project(":orca-core"))

implementation("org.jetbrains:annotations")
testImplementation("org.codehaus.groovy:groovy")
testImplementation("org.apache.groovy:groovy")
testImplementation("org.junit.jupiter:junit-jupiter-api")
testImplementation("org.assertj:assertj-core")
testImplementation("org.mockito:mockito-core:2.25.0")
Expand Down
2 changes: 1 addition & 1 deletion orca-keel/orca-keel.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ dependencies {
testImplementation("io.mockk:mockk")
testImplementation("io.strikt:strikt-jackson")
testImplementation("org.assertj:assertj-core")
testImplementation("org.codehaus.groovy:groovy")
testImplementation("org.apache.groovy:groovy")
testImplementation("org.junit.jupiter:junit-jupiter-api")
testImplementation("org.junit.jupiter:junit-jupiter-params")
testImplementation("com.github.tomakehurst:wiremock-jre8-standalone")
Expand Down
2 changes: 1 addition & 1 deletion orca-pipelinetemplate/orca-pipelinetemplate.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ dependencies {

testImplementation("org.slf4j:slf4j-simple")
testImplementation("org.assertj:assertj-core")
testImplementation("org.codehaus.groovy:groovy-json")
testImplementation("org.apache.groovy:groovy-json")
testImplementation("io.spinnaker.fiat:fiat-core:$fiatVersion")
testImplementation("io.spinnaker.kork:kork-retrofit")
testRuntimeOnly("net.bytebuddy:byte-buddy")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,13 +114,13 @@ class V1SchemaIntegrationSpec extends Specification {
}

if (it.filename.endsWith('-config.yml')) {
test.configuration = objectMapper.convertValue(yaml.load(it.file.text), TemplateConfiguration)
test.configuration = objectMapper.convertValue(yaml.load(it.getFile().text), TemplateConfiguration)
} else if (it.filename.endsWith('-expected.json')) {
test.expected = objectMapper.readValue(it.file, Map)
test.expected = objectMapper.readValue(it.getFile(), Map)
} else if (it.filename.endsWith('-request.json')) {
test.request = objectMapper.readValue(it.file, Map)
test.request = objectMapper.readValue(it.getFile(), Map)
} else {
test.template.add(objectMapper.convertValue(yaml.load(it.file.text), PipelineTemplate))
test.template.add(objectMapper.convertValue(yaml.load(it.getFile().text), PipelineTemplate))
}
}

Expand Down
2 changes: 1 addition & 1 deletion orca-queue/orca-queue.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,6 @@ dependencies {
testImplementation(project(":orca-queue-tck"))
testImplementation(project(":orca-queue-redis"))
testImplementation(project(":orca-echo"))
testImplementation("org.codehaus.groovy:groovy")
testImplementation("org.apache.groovy:groovy")
testImplementation("org.assertj:assertj-core")
}
2 changes: 1 addition & 1 deletion orca-retrofit/orca-retrofit.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ apply from: "$rootDir/gradle/groovy.gradle"
dependencies {
api("com.squareup.retrofit:retrofit")
api("com.squareup.retrofit:converter-jackson")
api("org.codehaus.groovy:groovy")
api("org.apache.groovy:groovy")
api("io.spinnaker.kork:kork-web")
api("com.jakewharton.retrofit:retrofit1-okhttp3-client")

Expand Down
2 changes: 1 addition & 1 deletion orca-web/orca-web.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ dependencies {
testImplementation("dev.minutest:minutest")
testImplementation("io.strikt:strikt-core")
testImplementation("io.mockk:mockk")
testImplementation("org.codehaus.groovy:groovy-json")
testImplementation("org.apache.groovy:groovy-json")
}

test {
Expand Down
2 changes: 1 addition & 1 deletion orca-webhook/orca-webhook.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ dependencies {
implementation("com.squareup.okhttp3:okhttp")
implementation("com.jakewharton.retrofit:retrofit1-okhttp3-client:1.1.0")
testImplementation("org.springframework:spring-test")
testImplementation("org.codehaus.groovy:groovy-json")
testImplementation("org.apache.groovy:groovy-json")
testRuntimeOnly("net.bytebuddy:byte-buddy")

implementation("io.spinnaker.fiat:fiat-api:$fiatVersion")
Expand Down

0 comments on commit 5bfd93a

Please sign in to comment.