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

Snapshot for standalone planner #378

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
65 changes: 65 additions & 0 deletions vars/deployPlannerSnapshot.groovy
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
#!/usr/bin/groovy

import java.util.regex.Pattern

def call(body) {
// evaluate the body block, and collect configuration into the object
def config = [:]
body.resolveStrategy = Closure.DELEGATE_FIRST
body.delegate = config
body()

def openShiftTemplate = config.openShiftTemplate
def originalImageName = config.originalImageName
def newImageName = config.newImageName
def deploymentName = config.githubRepo
def providerLabel = config.providerLabel ?: 'fabric8'
Copy link
Contributor

Choose a reason for hiding this comment

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

All local variables except this would be null. Maybe you forgot a config param in the function signature?

Copy link
Contributor

Choose a reason for hiding this comment

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

confuses me as well, and follows what is done in https://github.com/fabric8io/fabric8-pipeline-library/blob/master/vars/deployOpenShiftSnapshot.groovy#L11 Is there some magic involved?

Copy link
Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

@jaseemabid wonder why you thought all local variables would be null? wouldn't body() initialise these?

Copy link
Contributor

@sthaha sthaha Feb 14, 2018

Choose a reason for hiding this comment

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

@pranavgore09 @jaseemabid hopefully this may make sense to you ...

class Body {

  def closure =  {
    println delegate
    println "closure: foo: ${foo}"
    foo = 'barrrr'
    bar = 'baaazzz'
  }
}


def config = [ foo: "bar"]
def obj = new Body()
def body = obj.closure

body.delegate = config

println "main: foo: ${config.foo}"
println "main: bar: ${config.bar}"

body()

println "main: foo: ${config.foo}"
println "main: bar: ${config.bar}"

Output:

main: foo: bar                
main: bar: null               
class delegate                
closure: foo: bar             
main: foo: barrrr             
main: bar: baaazzz 

Copy link
Contributor

Choose a reason for hiding this comment

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

just in case you are wondering where I got this idea from :) - https://stackoverflow.com/a/23450944

def project = config.githubProject

def flow = new io.fabric8.Fabric8Commands()
def utils = new io.fabric8.Utils()
def openShiftProject = config.openShiftProject + '-' + utils.getRepoName()

if (!flow.isAuthorCollaborator("", project)){
currentBuild.result = 'ABORTED'
error 'Change author is not a collaborator on the project, aborting build until we support the [test] comment'
}
def yaml = flow.getUrlAsString(openShiftTemplate)
def originalImage = "- image: ${originalImageName}:(.*)"
def newImage = "- image: ${newImageName}"
def compiledYaml = Pattern.compile(originalImage).matcher(yaml).replaceFirst(newImage)

Copy link
Contributor

Choose a reason for hiding this comment

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

@pranavgore09 Its slightly odd that we use string matching and regular expressions to get data out of a structured data interchange format. I would have expected to see something like this.

def yaml = Yaml.parse(flow.getUrlAsString(openShiftTemplateURL))
yaml.services[1].image = config.newImage  // Whatever the real path is
def snapshot = yaml.toString()
// Remove the check below, because that wont fail now. 

I'm OK with this code, this might be written this way because of other nuances like groovy sandbox or something which we don't know.

Point being, this feels like a very poor groovy implementation of

wget openShiftTemplate > snapshot.yml
sed -i -e 's/oldImage/newImage/g' snapshot.yml

and that feels really odd.

Copy link
Contributor

Choose a reason for hiding this comment

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

It indeed has a method to parse yaml but @jaseemabid mentioned I feel this is consistent with the existing implementation so lets go ahead with this and fix both later as a separate issue.

if (!compiledYaml.contains(newImage)){
error "original image ${originalImage} not replaced with ${newImage} in yaml: \n ${compiledYaml}"
}
// cant use writeFile as we are facing following error
// Scripts not permitted to use staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods
sh "echo '${compiledYaml}' > snapshot.yml"

container('clients') {
try {
sh "oc get project ${openShiftProject} | grep Active"
} catch (err) {
echo "${err}"
sh "oc new-project ${openShiftProject}"
}

sh "oc process -n ${openShiftProject} -f ./snapshot.yml | oc apply -n ${openShiftProject} -f -"

sleep 10
// ok bad bad but there's a delay between DC's being applied and new pods being started. lets find a better way to do this looking at teh new DC perhaps?

waitUntil {
// wait until the pods are running has been deleted
try {
sh "oc get pod -l app=${deploymentName},provider=${providerLabel} -n ${openShiftProject} | grep Running"
echo "${deploymentName} pod is running"
return true
} catch (err) {
echo "waiting for ${deploymentName} to be ready..."
return false
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this waitUntill a tight loop? Do you think there should be a sleep if it fails? Is that inbuilt? If we add a sleep before the return false, we can remove the sleep 10 above?

}
}
return sh(script: "oc get route ${deploymentName} -o jsonpath=\"{.spec.host}\" -n ${openShiftProject}", returnStdout: true).toString().trim()
}
}