Skip to content

Add a scheduled CI job to flag supported platforms going EOL upstream. - #14581

Merged
tkatsoulas merged 3 commits into
netdata:masterfrom
Ferroin:auto-eol-issues
Feb 24, 2023
Merged

Add a scheduled CI job to flag supported platforms going EOL upstream.#14581
tkatsoulas merged 3 commits into
netdata:masterfrom
Ferroin:auto-eol-issues

Conversation

@Ferroin

@Ferroin Ferroin commented Feb 21, 2023

Copy link
Copy Markdown
Member
Summary

By default, it runs at 03:00 UTC every Monday, and checks the upstream EOL date for each platform we support that needs such checking. If the platform will be EOL upstream within the next 30 days, an issue is opened flagging the platform for removal from CI and our support document and auto-assigned to the agent SRE team members.

The workflow can also be manually triggered (mostly intended for testing).

Data about upstream EOL dates is retrieved from https://endoflife.date via their new public API. Happily, our own definition of what consititutes EOL for our purposes matches up 1:1 with how they categorize platforms as EOL, so the code handling this is relatively simple.

The original idea was to open a PR instead with the required changes, but correctly handling the required changes is actually nontrivial to automate, so I decided to just do an issue instead.

Test Plan

Confirm that actionlint flags no issues with the new workflow.

Beyond that, merge it and see what happens. This is not critical code, it can’t really break anything, and it’s trivial to remove it again if there are significant issues.

Additional Information

Credit to @tkatsoulas for the original idea on this one.

By default, it runs at 03:00 UTC every Monday, and checks the upstream
EOL date for each platform we support that needs such checking. If the
platform will be EOL upstream within the next 30 days, an issue is opened
flagging the platform for removal from CI and our support document and
auto-assigned to the agent SRE team members.

The workflow can also be manually triggered (mostly intended for
testing).

Data about upstream EOL dates is retrieved from https://endoflife.date
via their new public API. Happily, our own definition of what
consititutes EOL for our purposes matches up 1:1 with how they
categorize platforms as EOL, so the code handling this is relatively
simple.

The original idea was to open a PR instead with the required changes,
but correctly handling the required changes is actually nontrivial to
automate, so I decided to just do an issue instead.
@Ferroin Ferroin added area/ci no changelog Issues which are not going to be added to changelog labels Feb 21, 2023
@Ferroin
Ferroin marked this pull request as ready for review February 21, 2023 17:07
@Ferroin
Ferroin requested a review from tkatsoulas as a code owner February 21, 2023 17:07
@Ferroin
Ferroin requested a review from a team February 21, 2023 17:08
tkatsoulas
tkatsoulas previously approved these changes Feb 21, 2023
@tkatsoulas

Copy link
Copy Markdown
Contributor

LGTM! Clean and great implementation!

@thiagoftsm

Copy link
Copy Markdown
Contributor

@Ferroin , please, take a look in this report.

Comment thread .github/workflows/platform-eol-check.yml Outdated
@Ferroin

Ferroin commented Feb 22, 2023

Copy link
Copy Markdown
Member Author

@thiagoftsm That’s an issue of Codacy not recognizing new syntax that was added in Python 3.10 (see PEP 634 for a formal explanation of the syntax, and PEP 636 for a tutorial). The GHA runners are using Python 3.10, so we can use this syntax without issue in our CI (and I’m strongly in favor of doing so, as it often makes things much easier to read).

Comment on lines +83 to +88
count=$(gh issue list -R netdata/netdata -s all -S '${{ steps.title.outputs.title }} in:title' --json 'id' -q '. | length')
if [ "${count}" -ge 1 ]; then
echo 'exists=true' >> "${GITHUB_OUTPUT}"
else
echo 'exists=false' >> "${GITHUB_OUTPUT}"
fi

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Will exists be false if gh issue ... fails ([ "${count}" -ge 1 ] fails if count is unset).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The job will actually fail in that case due to the shell spitting out an error (it ends up trying to compare a string to a number).

This is actually the behavior we want here, because it makes sure we actually see that something went wrong (and it errs on the side of caution by not creating an issue at all in that case).

@ilyam8 ilyam8 Feb 22, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are you sure?

pve-deb-work ~ $ cat -p qq.sh
#!/bin/bash

if [ "${count}" -ge 1 ]; then
  echo 'exists=true'
else
  echo 'exists=false'
fi

pve-deb-work ~ $ ./qq.sh
./qq.sh: line 3: [: : integer expression expected
exists=false

pve-deb-work ~ $ echo $?
0

I did test it in CI and the job doesn't fail.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Hmm, I was under the impression that script blocks ran with set -e enabled. Apparently not.

@ilyam8 ilyam8 Feb 23, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Adding -e doesn't fix the problem (tested in my repo). We could add [ -z "${count}" ] && exit 1.

@ilyam8 ilyam8 Feb 23, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ok, I think I was wrong. The unhandled case: gh exit code is 0 but the command output is not a number. I think we can ignore it.

script blocks ran with set -e enabled

This is correct, but it seems that line 3: [: : integer expression expected in if doesn't stop code execution.

@Ferroin
Ferroin requested review from ilyam8 and tkatsoulas February 23, 2023 14:53
@tkatsoulas

Copy link
Copy Markdown
Contributor

@Ferroin merging it (3 2 1 .....)

@tkatsoulas
tkatsoulas merged commit a20b8c4 into netdata:master Feb 24, 2023
@Ferroin
Ferroin deleted the auto-eol-issues branch February 24, 2023 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/ci no changelog Issues which are not going to be added to changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants