feat(gce): support resourceManagerTags in instance template #6282
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Please delete the pull request instructions from the body of your pull request message. The instructions start with the line:
You can reopen your pull request when this has been addressed. |
edgarulg
changed the title
|
Are there any meaningful tests that can be added for this? |
I am still working on adding test for it but it is challenging because the BasicGoogleDeployHandler class is really hard to test :( |
* chore(dependencies): Autobump korkVersion
* refactor(dependency): replace groovy coordinates and code adjustment 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.
- Unpinning `io.rest-assured` to get in sync with kork.
- Removed unused import from class `com.netflix.spinnaker.clouddriver.oracle.deploy.op.DestroyOracleServerGroupAtomicOperationSpec.groovy`.
- While upgrading groovy 4.0.15:
Encounter below error during build process of clouddriver-appengine module and similar errors in clouddriver-aws, clouddriver-azure, clouddriver-core, clouddriver-google and clouddriver-oracle modules :
```
/clouddriver/clouddriver-appengine/build/tmp/compileGroovy/groovy-java-stubs/com/netflix/spinnaker/clouddriver/appengine/model/AppengineServerGroup.java:3: error: AppengineServerGroup is not abstract and does not override abstract method isDisabled() in ServerGroup
@groovy.transform.CompileStatic() @groovy.transform.EqualsAndHashCode(includes={"name","account"}) public class AppengineServerGroup
^
1 error
startup failed:
Compilation failed; see the compiler error output for details.
1 error
> Task :clouddriver-appengine:compileGroovy FAILED
```
To fix this issue added `isDisabled()` overriden method.
Encounter below error during test execution of clouddriver-aws module and similar errors in clouddriver-google module:
```
No such property: SUBNET_ID_OVERRIDE_TAG for class: com.netflix.spinnaker.clouddriver.aws.deploy.handlers.BasicAmazonDeployHandlerUnitSpec$1
groovy.lang.MissingPropertyException: No such property: SUBNET_ID_OVERRIDE_TAG for class: com.netflix.spinnaker.clouddriver.aws.deploy.handlers.BasicAmazonDeployHandlerUnitSpec$1
at app//groovy.lang.GroovyObject.getProperty(GroovyObject.java:50)
at app//groovy.lang.Closure.getPropertyTryThese(Closure.java:325)
at app//groovy.lang.Closure.getPropertyOwnerFirst(Closure.java:319)
at app//groovy.lang.Closure.getProperty(Closure.java:309)
at com.netflix.spinnaker.clouddriver.aws.deploy.handlers.BasicAmazonDeployHandler.copySourceAttributes_closure6(BasicAmazonDeployHandler.groovy:384)
at app//com.netflix.spinnaker.clouddriver.aws.deploy.handlers.BasicAmazonDeployHandler.copySourceAttributes(BasicAmazonDeployHandler.groovy:384)
at com.netflix.spinnaker.clouddriver.aws.deploy.handlers.BasicAmazonDeployHandlerUnitSpec.should copy subnet ids from source when available and not explicitly specified(BasicAmazonDeployHandlerUnitSpec.groovy:590)
```
In order to fix this issue, replace the static final variable with qualified name using classname. It may be related to this [issue](https://issues.apache.org/jira/browse/GROOVY-9386).
--------
Encounter below error during test execution of `AmazonCloudMetricProviderSpec.groovy` class in clouddriver-aws module:
```
No such property: args for class: com.netflix.spinnaker.clouddriver.aws.model.AmazonMetricDescriptor
groovy.lang.MissingPropertyException: No such property: args for class: com.netflix.spinnaker.clouddriver.aws.model.AmazonMetricDescriptor
at app//com.netflix.spinnaker.clouddriver.aws.model.AmazonMetricDescriptor.from(AmazonMetricDescriptor.groovy:41)
at app//com.netflix.spinnaker.clouddriver.aws.provider.view.AmazonCloudMetricProvider.getMetricDescriptor(AmazonCloudMetricProvider.groovy:66)
at com.netflix.spinnaker.clouddriver.aws.provider.view.AmazonCloudMetricProviderSpec.getMetrics returns a metric when one found(AmazonCloudMetricProviderSpec.groovy:74)
```
To fix this issue, created an explicit constructor and removed `@Immutable`.
----------
Encounter below error during test execution in clouddriver-core module:
```
No such property: completed for class: com.netflix.spinnaker.clouddriver.data.task.DefaultTaskStatus
groovy.lang.MissingPropertyException: No such property: completed for class: com.netflix.spinnaker.clouddriver.data.task.DefaultTaskStatus
at app//org.codehaus.groovy.runtime.ScriptBytecodeAdapter.unwrap(ScriptBytecodeAdapter.java:67)
at app//org.codehaus.groovy.vmplugin.v8.IndyGuardsFiltersAndSignatures.unwrap(IndyGuardsFiltersAndSignatures.java:163)
at app//org.codehaus.groovy.vmplugin.v8.IndyInterface.fromCache(IndyInterface.java:321)
at app//com.netflix.spinnaker.clouddriver.data.task.InMemoryTaskRepository.list(InMemoryTaskRepository.groovy:54)
at app//com.netflix.spinnaker.clouddriver.core.test.TaskRepositoryTck.testListRunningTasks(TaskRepositoryTck.java:107)
```
To fix this issue, replaced the fields with its getter method.
----------
Encounter below error during test execution of `ProjectClustersServiceSpec.groovy` class in clouddriver-core module, and similar error in clouddriver-titus module:
```
> Task :clouddriver-core:compileTestGroovy FAILED
startup failed:
/clouddriver/clouddriver-core/src/test/groovy/com/netflix/spinnaker/clouddriver/core/ProjectClustersServiceSpec.groovy: 495: The return type of boolean isDisabled() in com.netflix.spinnaker.clouddriver.core.ProjectClustersServiceSpec$TestServerGroup is incompatible with java.lang.Boolean in com.netflix.spinnaker.clouddriver.model.ServerGroup
. At [495:5] @ line 495, column 5.
boolean disabled
^
1 error
```
To fix this issue, replaced `boolean` primitive type to `Boolean` type.
----------
Encounter below error during compilation of clouddriver-google module:
```
/clouddriver/clouddriver-google/build/tmp/compileGroovy/groovy-java-stubs/com/netflix/spinnaker/clouddriver/google/deploy/exception/GoogleResourceNotFoundException.java:5: error: no suitable constructor found for GoogleOperationException(no arguments)
@groovy.transform.InheritConstructors() public class GoogleResourceNotFoundException
^
constructor GoogleOperationException.GoogleOperationException(String) is not applicable
(actual and formal argument lists differ in length)
constructor GoogleOperationException.GoogleOperationException(String,Throwable) is not applicable
(actual and formal argument lists differ in length)
/clouddriver/clouddriver-google/build/tmp/compileGroovy/groovy-java-stubs/com/netflix/spinnaker/clouddriver/google/deploy/exception/GoogleOperationTimedOutException.java:5: error: no suitable constructor found for GoogleOperationException(no arguments)
@groovy.transform.InheritConstructors() public class GoogleOperationTimedOutException
^
constructor GoogleOperationException.GoogleOperationException(String) is not applicable
(actual and formal argument lists differ in length)
constructor GoogleOperationException.GoogleOperationException(String,Throwable) is not applicable
(actual and formal argument lists differ in length)
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
Note: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.
3 errors
startup failed:
Compilation failed; see the compiler error output for details.
1 error
> Task :clouddriver-google:compileGroovy FAILED
```
To fix this issue, added a default constructor to `GoogleOperationException.groovy` class.
----------
Encounter below error during test execution in clouddriver-azure module:
```
Cannot invoke method isWarnEnabled() on null object
java.lang.NullPointerException: Cannot invoke method isWarnEnabled() on null object
at com.netflix.spinnaker.clouddriver.azure.resources.servergroup.ops.preprocessors.RegionsToRegionDescriptionPreProcessor.process_closure1(RegionsToRegionDescriptionPreProcessor.groovy:45)
at groovy.lang.Closure.call(Closure.java:433)
at groovy.lang.Closure.call(Closure.java:422)
at com.netflix.spinnaker.clouddriver.azure.resources.servergroup.ops.preprocessors.RegionsToRegionDescriptionPreProcessor.process(RegionsToRegionDescriptionPreProcessor.groovy:40)
at com.netflix.spinnaker.clouddriver.azure.resources.servergroup.ops.preprocessors.RegionsToRegionDescriptionPreProcessorSpec.should convert legacy descriptions(RegionsToRegionDescriptionPreProcessorSpec.groovy:33)
```
To fix this issue, explicitly defined a method to return `log` object inside `with{}` closure. It may be related to this [issue](https://issues.apache.org/jira/browse/GROOVY-8820)
---------
Co-authored-by: root <root@84926203b695>
Co-authored-by: j-sandy <30489233+j-sandy@users.noreply.github.com>
Co-authored-by: root <root@15a60bc3bd71>
# Conflicts: # clouddriver-google/clouddriver-google.gradle
edgarulg
requested review from
jeyrschabu,
ajordens,
cfieber,
zachsmith1,
clanesf and
german-muzquiz
as code owners
|
@jasonmcintosh @dbyron-sf @christosarvanitis I tried to add a test for the resourceManagerTags field I added to GCE but I am not able to test the |
|
@edgarulg I'm all for the refactor happening in a separate PR, especially if it can happen before this one. |
Hi @dbyron-sf. Normally I would be in full-throated agreement. This PR is a minimal change (adding one new field). Refactoring here would introduce more risk and have a greater chance of creating regressions. We are happy to take a crack at refactoring this to make it more testable soon after. |
This was referenced Sep 26, 2024
|
I am closing this PR because a lot of changes were added from master and it isn't clear what changes belongs to the |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Support
resourceManagerTagsin GCE. See: https://cloud.google.com/compute/docs/reference/rest/v1/instanceTemplates/insertAdds
resourceManagerTagsin the description operation and add it in the instanceTemplateProperties.Issue related: spinnaker/spinnaker#6931