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

chore: update local clusters setup on CD and Hourly #3727

Merged
merged 2 commits into from
Nov 20, 2023

Conversation

galibey
Copy link
Contributor

@galibey galibey commented Nov 20, 2023

  1. CD_DEV and CD_DEV_Mac - do not install k8s for local clusters, explicitly set installation type so it won't be broken when defaults change.
  2. Hourly - added local cluster to longevity tests, set it to every 2 hours instead of every hour to compensate for increased CI time.

@galibey galibey self-assigned this Nov 20, 2023
Copy link
Contributor

@digikata digikata left a comment

Choose a reason for hiding this comment

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

Approved but up to you if the macos-12 PROXY question needs further updates

Comment on lines -106 to -110
if [[ ${{ matrix.os }} == 'macos-12' ]]; then
export PROXY="--proxy-addr 127.0.0.1"
else
export PROXY=""
fi
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the macos-12 k8 not need the PROXY variable anymore? The other removal of PROXY below sets proxy always, so it's not clear if macos uniquely needs a PROXY or if all os combos will use it on some tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In CD_DEV we no longer have macos-12 as os, only ubuntu-latest, so I removed dead code.

in both CD_DEV and CD_DEV_Mac I removed the condition with PROXY and added the actual proxy argument to the start command on CD_DEV_Mac.

--proxy-addr 127.0.0.1 is always needed for macos, and is not needed for ubuntu.

Copy link
Contributor

@tjtelan tjtelan left a comment

Choose a reason for hiding this comment

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

Changes look fine. Curious if things will just work

@galibey galibey added this pull request to the merge queue Nov 20, 2023
Merged via the queue into master with commit 6dc55f8 Nov 20, 2023
102 checks passed
@galibey galibey deleted the chore/update-cd-dev branch November 21, 2023 05:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants