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

Scheduled AKS and EKS ci doesn't work due to empty version #1119

Open
orfeas-k opened this issue Oct 16, 2024 · 5 comments · May be fixed by #1122
Open

Scheduled AKS and EKS ci doesn't work due to empty version #1119

orfeas-k opened this issue Oct 16, 2024 · 5 comments · May be fixed by #1122
Labels
bug Something isn't working

Comments

@orfeas-k
Copy link
Contributor

Bug Description

When scheduled, the workflows do not work because they are given an empty bundle_version.

When scheduled, there are no inputs (as when run from a workflow_dispatch). IIUC, this results in the workflow to call the parse_versions.py script with an input "". The script expects no argument in order to fail, thus it doesn't fail and instead outputs bundle_versions=[""].

We need to refactor the preprocess script/job in order to account for cases without inputs.

To Reproduce

trigger without any inputs e.g. pull_request event or wait for scheduled action

Environment

GH runners

Relevant Log Output

n/a

Additional Context

No response

@orfeas-k orfeas-k added the bug Something isn't working label Oct 16, 2024
Copy link

Thank you for reporting us your feedback!

The internal ticket has been created: https://warthogs.atlassian.net/browse/KF-6457.

This message was autogenerated

@orfeas-k
Copy link
Contributor Author

orfeas-k commented Oct 16, 2024

I see this was a design decision in #1101

the default value in the parse_versions() function has been removed, so calling the script without any arguments raises an appropriate exception.

This however is not in sync with our expected scheduled behaviour, where we want to run the workflows for latest + the two supported versions.

@orfeas-k
Copy link
Contributor Author

orfeas-k commented Oct 16, 2024

After a quick sync with @mvlassis, we both agreed that default values should only be set in the workflow and not the script. With that in mind, we came up with the two options:

  1. Adding a step in preprocess input job, that will run only on schedule and then this will be passed to the script as an argument, when input.kf_version is empty:
    - name: Set env
      if: ${{ github.event_name == 'schedule' }}
      run: echo "BUNDLE_VERSION=1.9" >> $GITHUB_ENV
        
    - name: Process bundle versions
      id: process_bundle_versions
      run: python scripts/gh-actions/parse_versions.py "${{ inputs.bundle_version || env.BUNDLE_VERSION }}"
  1. Modify the workflows to enable workflow_call with the same inputs that workflow_dispatch has and then create a separate e.g. deploy-to-a/eks-schedule.yaml or scheduled.yaml workflow that will call the deploy-to-a/eks.yaml with the appropriate inputs (setting the default there).

@DnPlas
Copy link
Contributor

DnPlas commented Oct 17, 2024

I'm not sure I'd like to go with (2), it seems to be adding more maintenance to us. I think I prefer going with a hybrid of (1), but first, I think there are a couple things going on here:

  1. The python script is not raising when expected. I have noticed that the lines checking the length of the "input" is slightly incorrect. Right now we are doing the following in L15-L16:
    if len(sys.argv) < 1: # <--- this will always be False, as sys.argv[0] is always the name of the script
        raise Exception("No bundle versions given as input.")

In a pdb session:

$ python3 scripts/gh-actions/parse_versions.py ""
-> if len(sys.argv) < 1:
(Pdb) sys.argv
['scripts/gh-actions/parse_versions.py', '']
(Pdb) len(sys.argv)
2
(Pdb) sys.argv[0]
'scripts/gh-actions/parse_versions.py'

We have to change this to either sys.argv[1] or, slightly fancier, refactor with argparse.

  1. Since the ${{ inputs.bundle_version }} will be null for the scheduled workflow run, we don't need to run anything from the preprocess input job on schedule.

Proposal

I think we can:

  • Set an if in the preprocess-input job to run only on workflow dispatch. We have to make sure this won't run on schedule, otherwise (according to the preprocess script) it will raise, causing the job to fail.
  • Create another job called generate-bundle-versions that generates the bundle versions based on the condition that the wf is run on schedule or wf dispatch.
  • Use dependencies.yaml to parse the bundle versions on schedule

So it would look like:

# please be mindful the syntax can be slightly wrong

jobs:
  preprocess-input:
  if: ${{ github.event_name == 'wf dispatch' }} # we should check the actual name
  ...

  generate-bundle-versions-on-sched:
  if: ${{ github.event_name == 'schedule' }}
  run: |
    bundle_versions=$(yq '. | keys' .github/dependencies.yaml -o=json) # this gives you a json array
    echo "bundle_versions=$" >> "$GITHUB_OUTPUT"

  deploy-ckf-to-aks:
    runs-on: ubuntu-22.04
    strategy:
      matrix:
        bundle_version: ${{ fromJSON(jobs.preprocess-input.outputs.processed_bundle_versions) || fromJSON(jobs.generate-bundle-versions-on-sched.outputs.bundle_versions) }}

thoughts?

@mvlassis
Copy link
Contributor

@DnPlas That is a great suggestion, I will be implementing this (and crediting you :))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants