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 RELEASE_VERSION parameter dependency #64

Closed
sbose78 opened this issue Nov 8, 2018 · 7 comments
Closed

Fix RELEASE_VERSION parameter dependency #64

sbose78 opened this issue Nov 8, 2018 · 7 comments
Assignees
Labels
enhancement Enhancements

Comments

@sbose78
Copy link
Member

sbose78 commented Nov 8, 2018

def applyDefaults(provided=[:], templateParams) {
def params = [:]
def setParam = { key, compute ->
if (key in templateParams) {
params[key] = provided[key] ?: compute()
}
}
setParam('SUFFIX_NAME') { "-${env.BRANCH_NAME}".toLowerCase() }
setParam('SOURCE_REPOSITORY_REF') { Utils.shWithOutput(this, "git rev-parse --short HEAD") }
setParam('SOURCE_REPOSITORY_URL') { Utils.shWithOutput(this, "git config remote.origin.url") }
setParam('RELEASE_VERSION') { Utils.shWithOutput(this, "git rev-list --count HEAD") }
return params
}

This method makes it mandatory for the openshift template to have the RELEASE_VERSION parameter else there's a failure because of non-availability of meta

def required = ['ImageStream', 'DeploymentConfig', 'meta']
def res = Utils.mergeResources(args.resources)
def found = res.keySet()
def missing = required - found
if (missing) {
error "Missing mandatory build resources params: $missing; found: $found"
}
def tag = res.meta.tag
if (!tag) {
error "Missing mandatory metadata: tag"
}

Is this intended or should we have something like :

// returns a map of parameter name: value by choosing from provided if the
// key is present in templateParams or uses default value
def applyDefaults(provided=[:], templateParams) {

  def params = [:]
  def setParam = { key, compute ->
    if (key in templateParams) {
      params[key] = provided[key] ?: compute()
    }
   else{
     params[key]  = compute()
   }  
  }

  setParam('SUFFIX_NAME') { "-${env.BRANCH_NAME}".toLowerCase() }
  setParam('SOURCE_REPOSITORY_REF') { Utils.shWithOutput(this, "git rev-parse --short HEAD") }
  setParam('SOURCE_REPOSITORY_URL') { Utils.shWithOutput(this, "git config remote.origin.url") }
  setParam('RELEASE_VERSION') { Utils.shWithOutput(this, "git rev-list --count HEAD") }
  return params
}
sbose78 added a commit to sbose78/osio-pipeline that referenced this issue Nov 8, 2018
Validate fix for fabric8io#64 with a booster
@sbose78
Copy link
Member Author

sbose78 commented Nov 8, 2018

This would fail too if the param is absent.

+ oc process -f .openshiftio/application.yaml SUFFIX_NAME=-master SOURCE_REPOSITORY_REF=e61ed08 SOURCE_REPOSITORY_URL=https://github.com/sbose78/nodejs-rest-http-crud.git RELEASE_VERSION=140 -o yaml
unknown parameter name "RELEASE_VERSION"

@sbose78
Copy link
Member Author

sbose78 commented Nov 8, 2018

--ignore-unknown-parameters
openshift/origin#15230 should work though.

@hrishin
Copy link
Member

hrishin commented Nov 8, 2018

If a user doesn't provide RELEASE_VERSION parameter then processtemplate generates default value for it.

setParam('RELEASE_VERSION') { Utils.shWithOutput(this, "git rev-list --count HEAD") }

For a user its optional parameter however for pipeline process its a madentory parameter.

Ideally, it should not casue any issue. Let us check it once why its failing.

Does that answers the issue @sbose78?

@sbose78
Copy link
Member Author

sbose78 commented Nov 8, 2018

@hrishin if the user doesn't provide it, but the param is present in the template, only then it works. If the param is absent in the template,it errors out. I've pointed out the code responsible for that :)

@sbose78
Copy link
Member Author

sbose78 commented Nov 9, 2018

A good number of boosters would need to be updated to work with this.
nodeshift-starters/nodejs-rest-http-crud#116

@hrishin
Copy link
Member

hrishin commented Nov 9, 2018

Yes @sbose78. Thanks! for the PR. 👍

@hrishin hrishin changed the title Should TEMPLATE_VERSION be a mandatory parameter ? Fix RELEASE_VERSION parameter dependency Nov 13, 2018
@hrishin hrishin added the enhancement Enhancements label Nov 20, 2018
@pradeepto
Copy link

@hrishin Assuming that you are working on it and thus assigning it to you. Change if it is wrong.

pradeepitm12 pushed a commit to pradeepitm12/osio-pipeline that referenced this issue Nov 30, 2018
Missing mandatory field check in template before processing.

Resolve
 - fabric8io#64
pradeepitm12 pushed a commit to pradeepitm12/osio-pipeline that referenced this issue Nov 30, 2018
Missing mandatory field check in template before processing.

Resolve
 - fabric8io#64
pradeepitm12 pushed a commit to pradeepitm12/osio-pipeline that referenced this issue Dec 3, 2018
Missing mandatory field check in template before processing.

Resolve
 - fabric8io#64
pradeepitm12 pushed a commit to pradeepitm12/osio-pipeline that referenced this issue Dec 4, 2018
Missing mandatory field check in template before processing.

Resolve
 - fabric8io#64
pradeepitm12 pushed a commit to pradeepitm12/osio-pipeline that referenced this issue Dec 5, 2018
Missing mandatory field check in template before processing.

Resolve
 - fabric8io#64
pradeepitm12 pushed a commit to pradeepitm12/osio-pipeline that referenced this issue Dec 11, 2018
Missing mandatory field check in template before processing.

Resolve
 - fabric8io#64
pradeepitm12 pushed a commit to pradeepitm12/osio-pipeline that referenced this issue Dec 11, 2018
Missing mandatory field check in template before processing.

Resolve
 - fabric8io#64
pradeepitm12 pushed a commit to pradeepitm12/osio-pipeline that referenced this issue Dec 11, 2018
Missing mandatory field check in template before processing.

Resolve
 - fabric8io#64
@hrishin hrishin removed their assignment Dec 11, 2018
pradeepitm12 pushed a commit to pradeepitm12/osio-pipeline that referenced this issue Dec 12, 2018
Missing mandatory field check in template before processing.

Resolve
 - fabric8io#64
hrishin pushed a commit that referenced this issue Dec 13, 2018
If some mandatory parameter is missing is from the template, the pipeline 
build fails

This patch check for missing mandatory field in the template and validate it
before processing.

Resolve
 - #64
@hrishin hrishin closed this as completed Dec 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancements
Projects
None yet
Development

No branches or pull requests

4 participants