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

added tekton task for Code Engine Deploy #277 #128

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

balassree
Copy link

Added Tekton Task to deploy application to the Code Engine. This to be reviewed and verified, before merging the code

Copy link
Member

@seansund seansund left a comment

Choose a reason for hiding this comment

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

Bala, the task looks good. I have a couple of updates that need to be made first.

Additionally, you need to sign your commit(s):

  • git commit --amend --no-edit --signoff
  • git push --force-with-lease origin tektontask-ce

image: $(params.tools-image)
workingdir: $(params.source-dir)
env:
- name: NAMESPACE
Copy link
Member

Choose a reason for hiding this comment

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

Remove this environment variable. It isn't used by the task

- name: image-to
type: string
- name: tools-image
default: garagecatalyst/ibmcloud-dev:1.1.3
Copy link
Member

Choose a reason for hiding this comment

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

We have a newer version of this image hosted in quay.io. Can you update this to quay.io/ibmgaragecloud/ibmcloud-dev:v2.0.4?

metadata:
name: ibm-codeengine-deploy
spec:
params:
Copy link
Member

Choose a reason for hiding this comment

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

All the values except for the image url are provided via the codeengine-access secret and that limits the usability of the task. (I.e. only one app can be deployed per CI namespace.) At the very least, the app name should be a required parameter like the image. Add optional parameters for the other values to allow any of the defaults to be overridden for a particular application. This can be done by defining the parameter with a default value of "", testing the parameter value in the task, and using the default value if one is not provided.

- name: CE_DEPLOY
valueFrom:
secretKeyRef:
name: codeengine-access
Copy link
Member

Choose a reason for hiding this comment

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

This tasks expects a secret named codeengine-access to exist with a particular structure. If the secret exists but it is missing some of the fields it appears this task will fail (e.g. if CE_DEPLOY is set to true but CE_PROJECT_NAME is missing then ibmcloud ce project select -n ${CE_PROJECT_NAME} will fail).

Please provide a file named 12-ibm-codeengine-deploy.md in the tasks/ folder with information about the structure of the secret and instructions on how to create it. If necessary, you can add a shell script in the bin/ folder to help with the process. (I know this is something new but we will be doing the same for the other tasks as well.)

@balassree
Copy link
Author

Addressed the Review Comments, by changing the input parameter for the task from the CI Stages rather than from Secrets as per Comments. Added .md file for the task

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants