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

Improve make and development setup #41

Merged
merged 8 commits into from Jul 7, 2021
Merged

Improve make and development setup #41

merged 8 commits into from Jul 7, 2021

Conversation

MarcelMue
Copy link
Contributor

No description provided.

@MarcelMue MarcelMue self-assigned this Jul 6, 2021
- architect/run-tests-with-abs:
chart_dir: "./helm/policies-common"
app-build-suite_version: "v0.2.3"
app-build-suite_container_tag: "0.2.3"
additional_app-build-suite_flags: "--external-cluster-version $KUBERNETES_VERSION"
- run:
name: Export kind logs
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Export the logs so we have all the pod logs in circle.

command: |
./kubectl create -f https://raw.githubusercontent.com/kubernetes-sigs/cluster-api/release-0.3/config/crd/bases/cluster.x-k8s.io_clusters.yaml > /dev/null
./kubectl create -f https://raw.githubusercontent.com/kubernetes-sigs/cluster-api/release-0.3/config/crd/bases/cluster.x-k8s.io_machinedeployments.yaml > /dev/null
make setup
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Use make for local dev and ci now.

Copy link
Contributor

Choose a reason for hiding this comment

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

Neat

@@ -1,13 +1,17 @@
.PHONY: generate
generate:
./template.sh
./hack/template.sh
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Move scripts to the hack folder to clean up a little bit.

kubectl create --context kind-kyverno-cluster -f https://raw.githubusercontent.com/giantswarm/apiextensions/b9a6bed05850045530eaf8ba7fd01941de6c8525/helm/crds-common/templates/giantswarm.yaml
kubectl create --context kind-kyverno-cluster -f https://raw.githubusercontent.com/kubernetes-sigs/cluster-api/release-0.3/config/crd/bases/cluster.x-k8s.io_clusters.yaml
kubectl create --context kind-kyverno-cluster -f https://raw.githubusercontent.com/kubernetes-sigs/cluster-api/release-0.3/config/crd/bases/cluster.x-k8s.io_machinedeployments.yaml
kubectl create --context kind-kyverno-cluster -f https://raw.githubusercontent.com/kubernetes-sigs/cluster-api-provider-aws/main/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml
kubectl create --context kind-kyverno-cluster -f https://raw.githubusercontent.com/kubernetes-sigs/cluster-api-provider-aws/main/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusterroleidentities.yaml
kubectl create --context kind-kyverno-cluster -f https://raw.githubusercontent.com/kyverno/kyverno/main/definitions/release/install.yaml
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This needed to be reordered following upstream issue: kyverno/kyverno#2094

Copy link
Contributor

Choose a reason for hiding this comment

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

Does abs have logic to wait for kyverno to be up? If not, you could wait here. Something like https://github.com/giantswarm/coredns-warnlist-plugin/blob/f1d8ff108254e4f07968b718e933593de97ecd5d/.circleci/config.yml#L98

Copy link
Member

Choose a reason for hiding this comment

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

app-build-suite (soon to be replaced with app-test-suite for testing) usually waits until the app cr shows status deployed

https://github.com/giantswarm/app-test-suite/blob/master/app_test_suite/steps/base_test_runner.py#L315

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought about waiting here but currently it will just make the test run longer - kyverno is up way before the tests start.

The problem is that kyverno is not the app-cr and therefor we wait for the setup of the kyverno-policies.

All in all not a big deal currently.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The problem with the bug I discovered was, that kyverno does currently not reload CRDs after it is started. This caused kyverno to fail if it was ready before all the CRDs were applied.

@@ -34,9 +34,6 @@ spec:
- key: "{{ `{{` }} releaseVersion {{ `}}` }}"
operator: Equals
value: "20.0.0"
- key: "{{ `{{` }} request.object.metadata.labels.\"release.giantswarm.io/version\" {{ `}}` }}"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unrelated change which I messed up on main.

@MarcelMue MarcelMue requested a review from a team July 6, 2021 18:59
Copy link
Contributor

@stone-z stone-z left a comment

Choose a reason for hiding this comment

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

One comment, but looking cleaner already

command: |
./kubectl create -f https://raw.githubusercontent.com/kubernetes-sigs/cluster-api/release-0.3/config/crd/bases/cluster.x-k8s.io_clusters.yaml > /dev/null
./kubectl create -f https://raw.githubusercontent.com/kubernetes-sigs/cluster-api/release-0.3/config/crd/bases/cluster.x-k8s.io_machinedeployments.yaml > /dev/null
make setup
Copy link
Contributor

Choose a reason for hiding this comment

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

Neat

kubectl create --context kind-kyverno-cluster -f https://raw.githubusercontent.com/giantswarm/apiextensions/b9a6bed05850045530eaf8ba7fd01941de6c8525/helm/crds-common/templates/giantswarm.yaml
kubectl create --context kind-kyverno-cluster -f https://raw.githubusercontent.com/kubernetes-sigs/cluster-api/release-0.3/config/crd/bases/cluster.x-k8s.io_clusters.yaml
kubectl create --context kind-kyverno-cluster -f https://raw.githubusercontent.com/kubernetes-sigs/cluster-api/release-0.3/config/crd/bases/cluster.x-k8s.io_machinedeployments.yaml
kubectl create --context kind-kyverno-cluster -f https://raw.githubusercontent.com/kubernetes-sigs/cluster-api-provider-aws/main/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml
kubectl create --context kind-kyverno-cluster -f https://raw.githubusercontent.com/kubernetes-sigs/cluster-api-provider-aws/main/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusterroleidentities.yaml
kubectl create --context kind-kyverno-cluster -f https://raw.githubusercontent.com/kyverno/kyverno/main/definitions/release/install.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

Does abs have logic to wait for kyverno to be up? If not, you could wait here. Something like https://github.com/giantswarm/coredns-warnlist-plugin/blob/f1d8ff108254e4f07968b718e933593de97ecd5d/.circleci/config.yml#L98

KIND_VERSION: v0.10.0
KUBERNETES_VERSION: v1.19.7
KIND_VERSION: v0.11.1
KUBERNETES_VERSION: v1.21.1
Copy link
Member

Choose a reason for hiding this comment

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

You'll want to update the kubernetes version also in the .abs/main.yaml

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am still looking for a way to have a single source of truth for this - currently thinking the make file / env and then replacing in main.yaml.

Nothing seems beautiful though 😅

@@ -1,8 +1,9 @@
kind create cluster --name kyverno-cluster
Copy link
Member

Choose a reason for hiding this comment

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

This is missing the --image flag from ci

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should also be fixed now.

@MarcelMue MarcelMue merged commit 5a168f6 into main Jul 7, 2021
@MarcelMue MarcelMue deleted the improve-make branch July 7, 2021 08:11
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.

None yet

4 participants