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

Create CI infrastructure in GitHub Actions #337

Merged
merged 2 commits into from
Nov 28, 2022
Merged

Conversation

iblancasa
Copy link
Collaborator

Signed-off-by: Israel Blancas iblancas@redhat.com

What this PR does / why we need it:

  • Fixes CI is not run in PRs #335: CI is not run in PRs
  • Use GitHub Actions for the CI
  • Add a job for the unit tests (missing in the CircleCI automation)

Some notes

  • Create as a draft because I need to add a workflow to create releases

@iblancasa
Copy link
Collaborator Author

@kensipe, do you want me to remove the .circleci folder?

@kensipe
Copy link
Member

kensipe commented Feb 17, 2022

oh.. nice! We may want to give a heads up and align with kudo... but would limit this to < a week. I don't see any challenges but want to allow for alignment time.

and to answer the question.. yes! we should remove .circleci as part of this transition

Nice stuff!

@iblancasa
Copy link
Collaborator Author

Ok, nice.
Another question: do you want to create a GitHub workflow for the releases or do you want to keep the process explained in RELEASE.md?

@kensipe
Copy link
Member

kensipe commented Feb 17, 2022

it would be much nicer to have releases off github actions.
some details that may not be on the release doc. I build a docker build with buildx (multi-arch support) and push to docker hub. This is to support Operator SDK score card. It would be wonderful to automate all the things.

@kensipe
Copy link
Member

kensipe commented Feb 17, 2022

I see all the details are there either on the release doc.. or linked from it.

@kensipe
Copy link
Member

kensipe commented Feb 18, 2022

this is marked "Draft"... let me know when you're ready. It looks good. I expect we want to pull from circleci... is that the only thing left to change? I would suggest that we can have additional features (like the release process) on a separate PR. Thanks for this!

@kensipe
Copy link
Member

kensipe commented Apr 12, 2022

@iblancasa checking to see if this is ready?

@kensipe
Copy link
Member

kensipe commented Nov 14, 2022

@iblancasa do you think that is ready?

@kensipe
Copy link
Member

kensipe commented Nov 14, 2022

renewed interest in this... I can take it over if needed

@iblancasa
Copy link
Collaborator Author

iblancasa commented Nov 15, 2022

@kensipe I can finish it if you want. It seems @eddycharly started something in #423

@eddycharly
Copy link
Contributor

I didn't know about this PR, I made something super simple (only runs the linter and unit tests).
I failed to get integration tests working because TestEnv startup failure :(

@iblancasa
Copy link
Collaborator Author

Ok, this PR was originally to create the releases too. How about just fixing the PRs and create another PR for the releases?

@eddycharly
Copy link
Contributor

@iblancasa your PR looks great, are we going to merge it ?
Creating release can come in another PR IMHO.
This is critical to get CI running on PRs.

@iblancasa
Copy link
Collaborator Author

@eddycharly if everyone agrees, we can merge it.
I just pushed the branch with the latest changes. Also, removed the draft status.

@iblancasa iblancasa marked this pull request as ready for review November 17, 2022 15:41
@eddycharly
Copy link
Contributor

Outch all is red !

@eddycharly
Copy link
Contributor

This would be great to have !

@iblancasa
Copy link
Collaborator Author

@eddycharly I'm trying to fix it. It'll take a while...

Signed-off-by: Israel Blancas <iblancasa@gmail.com>
@iblancasa
Copy link
Collaborator Author

@kensipe @eddycharly ready for review. Sorry because I messed up something with a merge and I had to do some pushes...


sudo mkdir -p /usr/local/kubebuilder/bin

sudo curl -L https://dl.k8s.io/v1.16.4/kubernetes-server-linux-amd64.tar.gz -o /usr/local/kubebuilder/bin/kubernetes-server-linux-amd64.tar.gz
Copy link
Contributor

@eddycharly eddycharly Nov 17, 2022

Choose a reason for hiding this comment

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

This uses k8s 1.16 ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll guess so. I suggest installing etcd and apiserver using https://github.com/kubernetes-sigs/controller-runtime/tree/master/tools/setup-envtest in a follow-up MR - that can also upgrade to a supported K8s version.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not sure. I ported what the previous automation had. I think we can do the upgrade later as @erikgb suggested

Copy link
Member

Choose a reason for hiding this comment

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

agree with the team that we should upgrade the k8s version... but aligned with iblancasa that we can do that on the next PR

@kensipe
Copy link
Member

kensipe commented Nov 21, 2022

very interested in this PR and those related. I will be reviewing today and pushing to merge asap

Copy link
Contributor

@erikgb erikgb left a comment

Choose a reason for hiding this comment

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

In general LGTM, but I would prefer if the setup task were better integration into the Makefile, to allow it to be run on local developer workstation. But that can be improved in a follow-up PR. As mentioned in a comment, we also need to upgrade the K8s version. K8s 1.16 is so totally unsupported.

I also think we should remove the .circleci, as having multiple CI configs will appear confusing.

@iblancasa
Copy link
Collaborator Author

I agree about the upgrades and everything but, as I pointed in this comment, #337 (comment) this PRs is just porting the CI from one system to another.

@kensipe
Copy link
Member

kensipe commented Nov 28, 2022

agree with @erikgb however I'm good with landing this and removing circle in a separate PR. reviewing... however we will likely want to review the actual experience. Reviewing content now.

Copy link
Member

@kensipe kensipe left a comment

Choose a reason for hiding this comment

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

/lgtm. thank you so much @iblancasa ! I will be merging today. Based on tests, I will create a PR to remove circle. Following that we can look to bump versions to something more modern.


sudo mkdir -p /usr/local/kubebuilder/bin

sudo curl -L https://dl.k8s.io/v1.16.4/kubernetes-server-linux-amd64.tar.gz -o /usr/local/kubebuilder/bin/kubernetes-server-linux-amd64.tar.gz
Copy link
Member

Choose a reason for hiding this comment

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

agree with the team that we should upgrade the k8s version... but aligned with iblancasa that we can do that on the next PR

@eddycharly
Copy link
Contributor

More importantly I think we should swith to kind or something closer to a real cluster.
IMHO, running api server and etcd only is not enough.

@kensipe
Copy link
Member

kensipe commented Nov 28, 2022

@eddycharly we use api-server and etc for integration tests... we use kind for e2e tests... it seems we are covered there

@kensipe
Copy link
Member

kensipe commented Nov 28, 2022

Actions to follow:
#427
#428
#429

@kensipe kensipe merged commit 6f1ad38 into kudobuilder:main Nov 28, 2022
@eddycharly
Copy link
Contributor

we use kind for e2e tests

where does this happen ?

@eddycharly
Copy link
Contributor

In the e2e logs:

harness.go:220: started test environment (kube-apiserver and etcd) in 4.821556848s

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

Successfully merging this pull request may close these issues.

CI is not run in PRs
4 participants