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

ci: add node_image version to kind-action #313

Merged
merged 1 commit into from
Oct 19, 2023
Merged

Conversation

u5surf
Copy link
Contributor

@u5surf u5surf commented Oct 16, 2023

Fixes #311

https://github.com/grafana/k6-operator/actions/runs/6512492225/job/17690286747

Run helm/kind-action@v1.2.0
  with:
    node_image: kindest/node:v1.27.1
Installing kind...
Adding kind directory to PATH...
Installing kubectl...
Adding kubectl directory to PATH...
kind v0.11.1

The log said that we use the old kind despite kindest/node is newer one.
It can be fixed to select the old kindest/node version.
https://github.com/helm/kind-action/blob/v1.2.0/action.yml#L12-L13

@CLAassistant
Copy link

CLAassistant commented Oct 16, 2023

CLA assistant check
All committers have signed the CLA.

@@ -74,6 +74,8 @@ jobs:

- name: Create kind cluster
uses: helm/kind-action@v1.2.0
with:
node_image: "kindest/node:v1.21.1"
Copy link
Collaborator

@yorugac yorugac Oct 18, 2023

Choose a reason for hiding this comment

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

I don't think it's necessary to pass node_image here at all; because it appears that v1.21.1 is being passed by default, going by this CI job:
https://github.com/grafana/k6-operator/actions/runs/6512291825/job/17689689080

The real question is how to pass kindest/node:v1.27.1? By your comment, does it mean that kind-action must be of another version?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually, I've just realized that in my deps update PR, for non-Helm e2e test I needed to change kind version together with node:


🤔 Maybe it's the same in Helm?

Copy link
Contributor Author

@u5surf u5surf Oct 18, 2023

Choose a reason for hiding this comment

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

@yorugac
I confused that the commit 3057794 is HEAD now isn't it?

uses: helm/kind-action@v1.2.0

Would you like to use kind v0.20.0?
We should upgrade helm/kind-action because helm/kind-action@v1.2.0 used the old kind(v0.11.1) version by default following this document.

https://github.com/helm/kind-action/releases/tag/v1.2.0

Copy link
Collaborator

@yorugac yorugac Oct 18, 2023

Choose a reason for hiding this comment

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

@u5surf I've just been in the middle of testing the branch with that commit (PR #308 ) and merged it in just now. Sorry for confusion! So you'll need to do git rebase on main (it'll also remove failing unit tests from your PR).

Would you like to use kind v0.20.0?
We should upgrade helm/kind-action because helm/kind-action@v1.2.0 used the old kind(v0.11.1) version by default following this document.

Thanks for checking! Yes, I think let's go with kind-action upgrade then and 1.27.1 node image. Hopefully, it works in CI!

Copy link
Collaborator

@yorugac yorugac left a comment

Choose a reason for hiding this comment

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

LGTM; thanks for the update! This workflow requires manual dispatch at the moment, so I'll merge it in and start it.

@yorugac yorugac merged commit d375af0 into grafana:main Oct 19, 2023
5 checks passed
@yorugac
Copy link
Collaborator

yorugac commented Oct 19, 2023

CI workflow: https://github.com/grafana/k6-operator/actions/runs/6571812081
image

Seems all is good 🎉 Thank you @u5surf!

@u5surf
Copy link
Contributor Author

u5surf commented Oct 19, 2023

@yorugac
Thank you so much!
Can you add the hacktoberfest-accepted label on this PR?

https://hacktoberfest.com/participation/#pr-mr-details

@yorugac
Copy link
Collaborator

yorugac commented Oct 19, 2023

@u5surf done
The way I read it, it was optional if PR has been merged? 🤔 Anyway, it's there now!

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

Successfully merging this pull request may close these issues.

Investigate CI tests with Helm
3 participants