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

fix(pipelineTriggers): change buildNumber type from int to string #4316

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ class CreateBakeTaskSpec extends Specification {

@Shared
def buildInfo = new JenkinsBuildInfo(
"name", 0, "http://jenkins", "SUCCESS",
"name", null, "http://jenkins", "SUCCESS",
dbyron-sf marked this conversation as resolved.
Show resolved Hide resolved
[
new JenkinsArtifact("hodor_1.1_all.deb", "."),
new JenkinsArtifact("hodor-1.1.noarch.rpm", "."),
Expand All @@ -166,7 +166,7 @@ class CreateBakeTaskSpec extends Specification {

@Shared
def buildInfoWithUrl = new JenkinsBuildInfo(
"name", 0, "http://spinnaker.builds.test.netflix.net/job/SPINNAKER-package-echo/69/", "SUCCESS",
"name", null, "http://spinnaker.builds.test.netflix.net/job/SPINNAKER-package-echo/69/", "SUCCESS",
[
new JenkinsArtifact("hodor_1.1_all.deb", "."),
new JenkinsArtifact("hodor-1.1.noarch.rpm", "."),
Expand All @@ -176,7 +176,7 @@ class CreateBakeTaskSpec extends Specification {

@Shared
def buildInfoWithFoldersUrl = new JenkinsBuildInfo(
"name", 0, "http://spinnaker.builds.test.netflix.net/job/folder/job/SPINNAKER-package-echo/69/", "SUCCESS",
"name", null, "http://spinnaker.builds.test.netflix.net/job/folder/job/SPINNAKER-package-echo/69/", "SUCCESS",
[
new JenkinsArtifact("hodor_1.1_all.deb", "."),
new JenkinsArtifact("hodor-1.1.noarch.rpm", "."),
Expand All @@ -186,7 +186,7 @@ class CreateBakeTaskSpec extends Specification {

@Shared
def buildInfoWithUrlAndSCM = new JenkinsBuildInfo(
"name", 0, "http://spinnaker.builds.test.netflix.net/job/SPINNAKER-package-echo/69/", "SUCCESS",
"name", null, "http://spinnaker.builds.test.netflix.net/job/SPINNAKER-package-echo/69/", "SUCCESS",
[
new JenkinsArtifact("hodor_1.1_all.deb", "."),
new JenkinsArtifact("hodor-1.1.noarch.rpm", "."),
Expand All @@ -197,7 +197,7 @@ class CreateBakeTaskSpec extends Specification {

@Shared
def buildInfoWithUrlAndTwoSCMs = new JenkinsBuildInfo(
"name", 0, "http://spinnaker.builds.test.netflix.net/job/SPINNAKER-package-echo/69/", "SUCCESS",
"name", null, "http://spinnaker.builds.test.netflix.net/job/SPINNAKER-package-echo/69/", "SUCCESS",
[
new JenkinsArtifact("hodor_1.1_all.deb", "."),
new JenkinsArtifact("hodor-1.1.noarch.rpm", "."),
Expand All @@ -211,7 +211,7 @@ class CreateBakeTaskSpec extends Specification {

@Shared
def buildInfoWithUrlAndMasterAndDevelopSCMs = new JenkinsBuildInfo(
"name", 0, "http://spinnaker.builds.test.netflix.net/job/SPINNAKER-package-echo/69/", "SUCCESS",
"name", null, "http://spinnaker.builds.test.netflix.net/job/SPINNAKER-package-echo/69/", "SUCCESS",
[
new JenkinsArtifact("hodor_1.1_all.deb", "."),
new JenkinsArtifact("hodor-1.1.noarch.rpm", "."),
Expand All @@ -225,7 +225,7 @@ class CreateBakeTaskSpec extends Specification {

@Shared
def buildInfoNoMatch = new JenkinsBuildInfo(
"name", 0, "http://jenkins", "SUCCESS",
"name", null, "http://jenkins", "SUCCESS",
[
new JenkinsArtifact("hodornodor_1.1_all.deb", "."),
new JenkinsArtifact("hodor-1.1.noarch.rpm", "."),
Expand Down Expand Up @@ -378,7 +378,7 @@ class CreateBakeTaskSpec extends Specification {
given:
bakeConfig.buildInfo = contextInfo
def pipelineWithTrigger = pipeline {
trigger = new JenkinsTrigger("master", "job", 1, null)
trigger = new JenkinsTrigger("master", "job", '1', null)
if (triggerInfo != null) {
trigger.buildInfo = triggerInfo
}
Expand Down Expand Up @@ -431,7 +431,7 @@ class CreateBakeTaskSpec extends Specification {
given:
bakeConfig.buildInfo = contextInfo
def pipelineWithTrigger = pipeline {
trigger = new JenkinsTrigger("master", "job", 1, null)
trigger = new JenkinsTrigger("master", "job", '1', null)
if (triggerInfo != null) {
trigger.buildInfo = triggerInfo
}
Expand Down Expand Up @@ -482,7 +482,7 @@ class CreateBakeTaskSpec extends Specification {
]
]
def pipelineWithTrigger = pipeline {
trigger = new JenkinsTrigger("master", "job", 1, null)
trigger = new JenkinsTrigger("master", "job", '1', null)
trigger.buildInfo = buildInfo
stage {
type = "bake"
Expand Down Expand Up @@ -575,7 +575,7 @@ class CreateBakeTaskSpec extends Specification {
given:
bakeConfig.buildInfo = contextInfo
def pipelineWithTrigger = pipeline {
trigger = new JenkinsTrigger("master", "job", 1, null)
trigger = new JenkinsTrigger("master", "job", '1', null)
if (triggerInfo != null) {
trigger.buildInfo = triggerInfo
}
Expand Down Expand Up @@ -626,7 +626,7 @@ class CreateBakeTaskSpec extends Specification {
given:
bakeConfig.buildInfo = contextInfo
def pipelineWithTrigger = pipeline {
trigger = new JenkinsTrigger("master", "job", 1, null)
trigger = new JenkinsTrigger("master", "job", '1', null)
if (triggerInfo != null) {
trigger.buildInfo = triggerInfo
}
Expand Down Expand Up @@ -675,7 +675,7 @@ class CreateBakeTaskSpec extends Specification {
given:
bakeConfig.buildInfo = contextInfo
def pipelineWithTrigger = pipeline {
trigger = new JenkinsTrigger("master", "job", 1, null)
trigger = new JenkinsTrigger("master", "job", '1', null)
if (triggerInfo != null) {
trigger.buildInfo = triggerInfo
}
Expand Down Expand Up @@ -724,7 +724,7 @@ class CreateBakeTaskSpec extends Specification {
given:
bakeConfig.buildInfo = mapper.convertValue(contextInfo, Map)
def pipelineWithTrigger = pipeline {
trigger = new JenkinsTrigger("master", "job", 1, null)
trigger = new JenkinsTrigger("master", "job", '1', null)
if (triggerInfo != null) {
trigger.buildInfo = triggerInfo
}
Expand Down Expand Up @@ -773,7 +773,7 @@ class CreateBakeTaskSpec extends Specification {
given:
bakeConfig.buildInfo = contextInfo
def pipelineWithTrigger = pipeline {
trigger = new JenkinsTrigger("master", "job", 1, null)
trigger = new JenkinsTrigger("master", "job", '1', null)
if (triggerInfo != null) {
trigger.buildInfo = triggerInfo
}
Expand Down Expand Up @@ -824,7 +824,7 @@ class CreateBakeTaskSpec extends Specification {
given:
bakeConfig.buildInfo = contextInfo
def pipelineWithTrigger = pipeline {
trigger = new JenkinsTrigger("master", "job", 1, null)
trigger = new JenkinsTrigger("master", "job", '1', null)
if (triggerInfo != null) {
trigger.buildInfo = triggerInfo
}
Expand Down Expand Up @@ -878,7 +878,7 @@ class CreateBakeTaskSpec extends Specification {
given:
bakeConfig.buildInfo = contextInfo
def pipelineWithTrigger = pipeline {
trigger = new JenkinsTrigger("master", "job", 1, null)
trigger = new JenkinsTrigger("master", "job", '1', null)
if (triggerInfo != null) {
trigger.buildInfo = triggerInfo
}
Expand Down Expand Up @@ -932,7 +932,7 @@ class CreateBakeTaskSpec extends Specification {
given:
bakeConfig.buildInfo = contextInfo
def pipelineWithTrigger = pipeline {
trigger = new JenkinsTrigger("master", "job", 1, null)
trigger = new JenkinsTrigger("master", "job", '1', null)
if (triggerInfo != null) {
trigger.buildInfo = triggerInfo
}
Expand Down Expand Up @@ -1282,9 +1282,9 @@ class CreateBakeTaskSpec extends Specification {

where:
triggerConfig | queryParameter
[type: "jenkins", master: "master", job: "job", buildNumber: 1, rebake: true] | "1"
[type: "jenkins", master: "master", job: "job", buildNumber: 1, rebake: false] | null
[type: "jenkins", master: "master", job: "job", buildNumber: 1] | null
[type: "jenkins", master: "master", job: "job", buildNumber: '1', rebake: true] | "1"
[type: "jenkins", master: "master", job: "job", buildNumber: '1', rebake: false] | null
[type: "jenkins", master: "master", job: "job", buildNumber: '1'] | null
}

def "properly resolves package artifacts"() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import static com.netflix.spinnaker.orca.test.model.ExecutionBuilder.stage
class TitusAmazonServerGroupCreatorDecoratorSpec extends Specification {
def "should find image id from properties file"() {
given:
JenkinsTrigger jenkinsTrigger = new JenkinsTrigger("master", "job", 1, null)
JenkinsTrigger jenkinsTrigger = new JenkinsTrigger("master", "job", "1", null)
jenkinsTrigger.properties.put("imageName", "imageFromProperties")

def pipeline = pipeline {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ class FindImageFromClusterTaskSpec extends Specification {
imageId : "ami-012",
imageName : "ami-012-name",
image : [imageId: "ami-012", name: "ami-012-name", foo: "bar"],
buildInfo : [job: "foo-build", buildNumber: 1]
buildInfo : [job: "foo-build", buildNumber: '1']
]]
]

Expand All @@ -95,7 +95,7 @@ class FindImageFromClusterTaskSpec extends Specification {
imageId : "ami-234",
imageName : "ami-234-name",
image : [imageId: "ami-234", name: "ami-234-name", foo: "baz"],
buildInfo : [job: "foo-build", buildNumber: 1]
buildInfo : [job: "foo-build", buildNumber: '1']
]]
]
}
Expand Down Expand Up @@ -135,7 +135,7 @@ class FindImageFromClusterTaskSpec extends Specification {
imageId : "ami-012",
imageName : "ami-012-name",
image : [imageId: "ami-012", name: "ami-012-name", foo: "bar"],
buildInfo : [job: "foo-build", buildNumber: 1]
buildInfo : [job: "foo-build", buildNumber: '1']
]]
]

Expand Down Expand Up @@ -239,7 +239,7 @@ class FindImageFromClusterTaskSpec extends Specification {
imageId : "ami-012",
imageName : "ami-012-name-ebs",
image : [imageId: "ami-012", name: "ami-012-name-ebs", foo: "bar"],
buildInfo : [job: "foo-build", buildNumber: 1]
buildInfo : [job: "foo-build", buildNumber: '1']
]]
]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ class AppEngineBranchFinderSpec extends Specification {
@Unroll
def "(jenkins trigger) should resolve branch, using regex (if provided) to narrow down options"() {
given:
def trigger = new JenkinsTrigger("Jenkins", "poll_git_repo", 1, null)
trigger.buildInfo = new JenkinsBuildInfo("poll_git_repo", 1, "http://jenkins", "SUCCESS", [], scm)
def trigger = new JenkinsTrigger("Jenkins", "poll_git_repo", '1', null)
trigger.buildInfo = new JenkinsBuildInfo("poll_git_repo", '1', "http://jenkins", "SUCCESS", [], scm)

def operation = [
trigger: [
Expand All @@ -99,8 +99,8 @@ class AppEngineBranchFinderSpec extends Specification {
@Unroll
def "(jenkins trigger) should throw appropriate error if method cannot resolve exactly one branch"() {
given:
def trigger = new JenkinsTrigger("Jenkins", "poll_git_repo", 1, null)
trigger.buildInfo = new JenkinsBuildInfo("poll_git_repo", 1, "http://jenkins", "SUCCESS", [], scm)
def trigger = new JenkinsTrigger("Jenkins", "poll_git_repo", '1', null)
trigger.buildInfo = new JenkinsBuildInfo("poll_git_repo", '1', "http://jenkins", "SUCCESS", [], scm)

def operation = [
trigger : [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,14 @@ class OortHelperSpec extends Specification {
"region": "us-west-2",
"asg": { "createdTime": 12344, "suspendedProcesses": [{"processName": "AddToLoadBalancer"}] },
"image": { "imageId": "ami-012", "name": "ami-012" },
"buildInfo": { "job": "foo-build", "buildNumber": 1 },
"buildInfo": { "job": "foo-build", "buildNumber": "1" },
"instances": [ { "id": 1 }, { "id": 2 } ]
},{
"name": "myapp-v003",
"region":"us-west-2",
"asg": { "createdTime": 23456, "suspendedProcesses": [] },
"image": { "imageId": "ami-234", "name": "ami-234" },
"buildInfo": { "job": "foo-build", "buildNumber": 1 },
"buildInfo": { "job": "foo-build", "buildNumber": "1" },
"instances": [ { "id": 1 } ]
}]
}
Expand All @@ -67,7 +67,7 @@ class OortHelperSpec extends Specification {
"region": "us-west-2",
"asg": { "createdTime": 12344, "suspendedProcesses": [{"processName": "AddToLoadBalancer"}] },
"image": { "imageId": "ami-012", "name": "ami-012" },
"buildInfo": { "job": "foo-build", "buildNumber": 1 },
"buildInfo": { "job": "foo-build", "buildNumber": "1" },
"instances": [ { "instanceId": 1, "health" : [{"healthCheckUrl" : "http://foo/bar"}, {"status": "UP"}] }, { "instanceId": 2, "health" : [{"healthCheckUrl" : "http://foo2/bar2"}, {"status": "UP"}] } ]
}]
}
Expand All @@ -92,7 +92,7 @@ class OortHelperSpec extends Specification {
"region": "us-west-2",
"asg": { "createdTime": 12344, "suspendedProcesses": [{"processName": "AddToLoadBalancer"}] },
"image": { "imageId": "ami-012", "name": "ami-012" },
"buildInfo": { "job": "foo-build", "buildNumber": 1 },
"buildInfo": { "job": "foo-build", "buildNumber": "1" },
"instances": [ { "instanceId": 1, "health" : [{"healthCheckUrl" : "http://foo/bar"}, {"status": "DOWN"}] },
{ "instanceId": 2, "health" : [{"healthCheckUrl" : "http://foo2/bar2"}, {"status": "UP"}] },
{ "instanceId": 3, "health" : [{"healthCheckUrl" : "http://foo2/bar3"}] },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class ParallelDeployStageSpec extends Specification {
def "should build contexts corresponding to cluster configuration(s)"() {
given:
def pipeline = pipeline {
trigger = new JenkinsTrigger("master", "job", 1, null)
trigger = new JenkinsTrigger("master", "job", "1", null)
application = "orca"
}
def bakeStage = new StageExecutionImpl(pipeline, "deploy", "Deploy!", stageContext)
Expand All @@ -57,7 +57,7 @@ class ParallelDeployStageSpec extends Specification {
def "pipeline strategy should #data.scenario"() {
given:
def parentPipeline = pipeline {
trigger = new JenkinsTrigger("master", "job", 1, null)
trigger = new JenkinsTrigger("master", "job", "1", null)
application = "orca"
stage {
name = "parent stage"
Expand Down
6 changes: 3 additions & 3 deletions orca-clouddriver/src/test/resources/pipelinetrigger.json
Original file line number Diff line number Diff line change
Expand Up @@ -11072,12 +11072,12 @@
"master": "mimir",
"job": "SPIN-TASKS",
"queuedBuild": "173601",
"buildNumber": 206809,
"buildNumber": "206809",
"buildInfo": {
"building": false,
"fullDisplayName": "SPIN-TASKS #206809",
"name": "SPIN-TASKS",
"number": 206809,
"number": "206809",
"duration": 13190,
"timestamp": "1510187419430",
"result": "SUCCESS",
Expand Down Expand Up @@ -11109,7 +11109,7 @@
"building": false,
"fullDisplayName": "SPIN-TASKS #206809",
"name": "SPIN-TASKS",
"number": 206809,
"number": "206809",
"duration": 13190,
"timestamp": "1510187419430",
"result": "SUCCESS",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ abstract class PipelineExecutionRepositoryTck<T extends ExecutionRepository> ext
def pipeline = pipeline {
application = "orca"
name = "dummy-pipeline"
trigger = new JenkinsTrigger("master", "job", 1, null)
trigger = new JenkinsTrigger("master", "job", '1', null)
stage {
type = "one"
context = [foo: "foo"]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package com.netflix.spinnaker.orca.pipeline.model

abstract class BuildInfo<A>(
open val name: String?,
open val number: Int,
open val number: String?,
dbyron-sf marked this conversation as resolved.
Show resolved Hide resolved
open val url: String?,
open val result: String?,
open val artifacts: List<A>? = emptyList(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ class ConcourseBuildInfo
@JsonCreator
constructor(
@param:JsonProperty("name") override val name: String?,
@param:JsonProperty("number") override val number: Int,
@param:JsonProperty("number") override val number: String?,
@param:JsonProperty("url") override val url: String?,
@param:JsonProperty("result") override val result: String?,
@param:JsonProperty("artifacts") override val artifacts: List<JenkinsArtifact>?,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ data class JenkinsTrigger
private var isStrategy: Boolean = false,
val master: String,
val job: String,
val buildNumber: Int,
val buildNumber: String?,
val propertyFile: String?
) : Trigger {

Expand Down Expand Up @@ -102,7 +102,7 @@ class JenkinsBuildInfo
@JsonCreator
constructor(
@param:JsonProperty("name") override val name: String?,
@param:JsonProperty("number") override val number: Int,
@param:JsonProperty("number") override val number: String?,
@param:JsonProperty("url") override val url: String?,
@param:JsonProperty("result") override val result: String?,
@param:JsonProperty("artifacts") override val artifacts: List<JenkinsArtifact>?,
Expand All @@ -115,7 +115,7 @@ constructor(
@JvmOverloads
constructor(
name: String,
number: Int,
number: String?,
url: String,
result: String,
artifacts: List<JenkinsArtifact> = emptyList(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ class TriggerDeserializer :
get("strategy")?.booleanValue() == true,
get("master").textValue(),
get("job").textValue(),
get("buildNumber").intValue(),
get("buildNumber").textValue(),
get("propertyFile")?.textValue()
).apply {
buildInfo = get("buildInfo")?.parseValue(parser)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,9 @@ public boolean tryToExtractBuildDetails(BuildInfo<?> buildInfo, Map<String, Obje
if (buildInfo == null || request == null) {
return false;
}
if (buildInfo.getUrl() != null && buildInfo.getName() != null && buildInfo.getNumber() > 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Aaah, here's the place that was treating a number <= 0 as missing...

if (buildInfo.getUrl() != null
&& buildInfo.getName() != null
&& buildInfo.getNumber() != null) {
Map<String, Object> copyRequest = new HashMap<>();
copyRequest.put("buildInfoUrl", buildInfo.getUrl());
copyRequest.put("job", buildInfo.getName());
Expand Down
Loading