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

feat: CI pipeline to build, package, test, and deploy each merged PR #912

Merged
merged 3 commits into from
Oct 11, 2024
Merged
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
62 changes: 0 additions & 62 deletions .github/workflows/container-latest.yaml

This file was deleted.

102 changes: 102 additions & 0 deletions .github/workflows/latest-release.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
name: latest-release

# Build "latest" container, run e2e tests against it, and deploy it to
# https://trustify-latest-staging.apps.cluster.trustification.rocks/

on:
push:
branches:
- main
workflow_dispatch:

concurrency: latest-release

env:
IMAGE_NAME: trustd
IMAGE_TAG: latest
OPENSHIFT_NAMESPACE: trustify-latest
OPENSHIFT_SERVER: https://api.cluster.trustification.rocks:6443
APP_NAME: staging

jobs:
build:
uses: ./.github/workflows/build-binary.yaml
with:
version: latest

publish:
needs: [ build ]
runs-on: ubuntu-22.04

outputs:
image: ${{ steps.push.outputs.registry-path }}

permissions:
contents: write
packages: write
id-token: write
attestations: write

steps:

- name: Checkout
uses: actions/checkout@v4

- uses: actions/download-artifact@v4
with:
path: ${{ github.workspace }}/download

- name: Display downloaded content
run: ls -R ${{ github.workspace }}/download

# Build the container

- uses: ./.github/actions/build-container
with:
image_name: ${{ env.IMAGE_NAME }}
image_tag: ${{ env.IMAGE_TAG }}

# Push to ghcr.io

- name: Push to ghcr.io
id: push
uses: redhat-actions/push-to-registry@v2
with:
image: ${{ env.IMAGE_NAME }}
tags: ${{ env.IMAGE_TAG }}
registry: ghcr.io/${{ github.repository_owner }}
username: ${{ github.repository_owner }}
password: ${{ secrets.GITHUB_TOKEN }}

e2e-test:
needs: publish
uses: trustification/trustify-ci/.github/workflows/global-ci.yml@main
with:
server_image: ${{ needs.publish.outputs.image }}
run_api_tests: true
run_ui_tests: true

deploy:
if: ${{ github.repository == 'trustification/trustify' }}
runs-on: ubuntu-22.04
needs:
- publish
- e2e-test
Copy link
Member

@carlosthe19916 carlosthe19916 Oct 10, 2024

Choose a reason for hiding this comment

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

@jcrossley3

  1. I wanted to point our attention to the fact that we will deploy the code only if the e2e tests passed. I think we should deploy it regardless of the e2e pass or not. Actually, I think the e2e test might or might not fail. Just yesterday you faced an e2e error due to the fact that the the UI of the backend had an outdated version. But I do not hold a strong opinion so I'll leave that to you.
  2. I thought the idea was to deploy the downstream version of Trustify not the upstream one. That might be just me not getting the requirement correctly though. In any case this is a good milestone!
  3. I created this issue Do not delete the NS and let the user do it by himself trustify-operator#34 in regards to the fact that you made the trustify-operator scripts to delete the namespace. I think that is a dangerous thing to be set in the trustify-operator scripts and the action of deletion should be done by the user itself (a conscious decision). So Just like you are using redhat-actions/oc-login@v1 to login in OCP you should delete the Namepace in a command defined in this workflow.
  • oc login (uses: redhat-actions/oc-login@v1)
  • oc delete project MY_PROJECT
  • Install the oprator uses: trustification/trustify-operator/.github/actions/install-trustify@main

Copy link
Contributor Author

@jcrossley3 jcrossley3 Oct 10, 2024

Choose a reason for hiding this comment

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

  1. I wanted to point our attention to the fact that we will deploy the code only if the e2e tests passed. I think we should deploy it regardless of the e2e pass or not. Actually, I think the e2e test might or might not fail. Just yesterday you faced an e2e error due to the fact that the the UI of the backend had an outdated version. But I do not hold a strong opinion so I'll leave that to you.

Interesting. I think of a CI pipeline as sequential (build, package, test, deploy, etc), failing immediately if any step fails. I like knowing that the thing deployed has passed all our tests, at least the ones known at the time. In the case of the failed ui tests yesterday, we'd only know about that after merging a PR in this repo, which isn't great, but at least we'd know about it eventually, in an automated way.

But if there's a majority opinion either way, I'm happy to abide.

  1. I thought the idea was to deploy the downstream version of Trustify not the upstream one. That might be just me not getting the requirement correctly though. In any case this is a good milestone!

Easy enough to do -- now that I know how -- but right now both upstream and downstream are identical, except for the konflux config, which isn't buying us anything yet.

Personally, I like for the deployment of the latest thing merged to come out of the repo that merged it.

  1. I created this issue Do not delete the NS and let the user do it by himself trustify-operator#34 in regards to the fact that you made the trustify-operator scripts to delete the namespace. I think that is a dangerous thing to be set in the trustify-operator scripts and the action of deletion should be done by the user itself (a conscious decision). So Just like you are using redhat-actions/oc-login@v1 to login in OCP you should delete the Namepace in a command defined in this workflow.

Yeah, I commented on that issue. Deleting the namespace was a hack that gave me a crude, idempotent behavior. And I didn't think it would affect anything you were currently using that script for, because you're always using it in a "virgin" minikube. With a long-lived OCP instance, we don't have the luxury of assuming we can install everything every time.

  • oc login (uses: redhat-actions/oc-login@v1)
  • oc delete project MY_PROJECT
  • Install the oprator uses: trustification/trustify-operator/.github/actions/install-trustify@main

To be clear, I don't want to delete the namespace. I want the operator to install a new version of trustify into that namespace. This would significantly reduce the time the app is down. This is the way operators should work, imo, but I need help refactoring that script to achieve that behavior.

Copy link
Member

@carlosthe19916 carlosthe19916 Oct 10, 2024

Choose a reason for hiding this comment

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

To be clear, I don't want to delete the namespace. I want the operator to install a new version of trustify into that namespace. This would significantly reduce the time the app is down. This is the way operators should work, imo, but I need help refactoring that script to achieve that behavior.

Agree. Deleting the NS also means deleting the whole data that was before. I don't want to block your PR, it has been approved, feel free to merge it. But it will be worth investigating how to achieve what you mentioned. Not sure I have too much time to support you in that journey though, but I'll try to find some free time to also suggest some solutions.

Copy link
Member

Choose a reason for hiding this comment

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

wait, I'll mention what I mentioned somewhere in other thread:

This is how Operators work.

  • Step 1: The operator is released
  • Step 2: you install your operator in OCP
  • Step 3: the team releases a new version of the backend (which included the UI) every night.
  • Step 4: each night the a new version of the operator is realeased
  • Step 5: OLM detects there is a new operator released and automatically updates the container images used in previous release and updates the application to the latest one.

Important to note that the Operator is installed only once in STEP 2.

This is how a real application receives updates when they install an operator. E.g. Today my company installs the Keycloak Operator in my OCP instance and in 1 month the Keycloak teams releases a new version of Keycloak (also the operator). ME as a company should not move a finger to receive updates from Keycloak as my operator, through OLM, should update the Keycloak version as soon as there is a new one.

Copy link
Member

Choose a reason for hiding this comment

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

That magic happens when you install an operator and enable the AUTOMATIC upgrades monitor:

ocp-operator-install-details1

That way, OLM does the magic and you don't need to actually deploy the whole thing after each commit.

But for that to happen a requirement would be:

  • The deployment is done nightly and not after each PR
  • Nightly a new version is released under a K8s channel development and it takes the latest version of container

That is just an idea, feel free to discard it :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That all sounds good to me, and resonates with what I recall from working with operators in the early days (pre bundles) back when they rarely worked. 😉

  • The deployment is done nightly and not after each PR

Why? What does the frequency matter?

Copy link
Member

Choose a reason for hiding this comment

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

Why? What does the frequency matter?

I think yes. But don't take my word for granted, I can be wrong.

For the automatic updates to work 2 things need to happen. A new container version of this repository needs to be created. And second, a new version of the trustify-operator needs to be released at https://github.com/redhat-openshift-ecosystem/community-operators-prod/pulls .

Since we have those 2 conditions, releasing for each commit would mean:


steps:

- name: Log in and set context
uses: redhat-actions/oc-login@v1
with:
openshift_server_url: ${{ env.OPENSHIFT_SERVER }}
openshift_token: ${{ secrets.OPENSHIFT_TOKEN }}

- name: Install trustify
uses: trustification/trustify-operator/.github/actions/install-trustify@main
env:
SERVER_IMAGE: ${{ needs.publish.outputs.image }}
with:
operator-bundle-image: ghcr.io/trustification/trustify-operator-bundle:latest
trustify-cr: '{"kind":"Trustify","apiVersion":"org.trustify/v1alpha1","metadata":{"name":"${{ env.APP_NAME }}"},"spec":{"serverImage":"${{ env.SERVER_IMAGE }}"}}'
namespace: ${{ env.OPENSHIFT_NAMESPACE }}
app-name: ${{ env.APP_NAME }}