Manual test feedbacks #3
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
summerwind
reviewed
Mar 7, 2020
Contributor
summerwind
left a comment
summerwind
left a comment
There was a problem hiding this comment.
Thank you for updating README!
I put a comment to solve workaround.
I had observed athe exact issue seen for the 4th option described in elastic/cloud-on-k8s#1822, which resulted in actions-runner-controller is unable to create nor update runners. This fixes that. I've also updated README to introduce RunnerDeployment and manually tested it to work after the fix. --- `actions-runner-controller` has been failing while creating and updating runners: ``` 2020-03-05T11:05:16.610+0900 ERROR controllers.Runner Failed to update runner {"runner": "default/example-runner", "error": "Runner.actions.summerwind.dev \"example-runner\" is invalid: []: Invalid value: map[string]interface {}{\"apiVersion\":\"actions.summerwind.dev/v1alpha1\", \"kind\":\"Runner\", \"metadata\":map[string]interface {}{\"creationTimestamp\":\"2020-03-05T02:05:16Z\", \"finalizers\":[]interface {}{\"runner.actions.summerwind.dev\"}, \"generation\":2, \"name\":\"example-runner\", \"namespace\":\"default\", \"resourceVersion\":\"911496\", \"selfLink\":\"/apis/actions.summerwind.dev/v1alpha1/namespaces/default/runners/example-runner\", \"uid\":\"48b62d07-ff2c-42d6-878c-d3f951202209\"}, \"spec\":map[string]interface {}{\"env\":interface {}(nil), \"image\":\"\", \"repository\":\"mumoshu/actions-runner-controller-ci\"}}: validation failure list:\nspec.env in body must be of type array: \"null\""} github.com/go-logr/zapr.(*zapLogger).Error /Users/c-ykuoka/go/pkg/mod/github.com/go-logr/zapr@v0.1.0/zapr.go:128 github.com/summerwind/actions-runner-controller/controllers.(*RunnerReconciler).Reconcile /Users/c-ykuoka/p/actions-runner-controller/controllers/runner_controller.go:88 ``` This seems like the exact issue seen in the 4th option in elastic/cloud-on-k8s#1822 I also observed the same issue is failing while the runnerset controller is trying to create/update runners: ``` Also while creating runner in the runnerset controller: 2020-03-05T11:15:01.223+0900 ERROR controller-runtime.controller Reconciler error {"controller": "runnerset", "request": "default/example-runnerset", "error": "Runner.actions.summerwind.dev \"example-runnersetgp56m\" is invalid: []: Invalid value: map[string]interface {}{\"apiVersion\":\"actions.summerwind.dev/v1alpha1\", \"kind\":\"Runner\", \"metadata\":map[string]interface {}{\"creationTimestamp\":\"2020-03-05T02:15:01Z\", \"generateName\":\"example-runnerset\", \"generation\":1, \"name\":\"example-runnersetgp56m\", \"namespace\":\"default\", \"ownerReferences\":[]interface {}{map[string]interface {}{\"apiVersion\":\"actions.summerwind.dev/v1alpha1\", \"blockOwnerDeletion\":true, \"controller\":true, \"kind\":\"RunnerSet\", \"name\":\"example-runnerset\", \"uid\":\"e26f7d01-3168-496d-931b-8e6f97b776ea\"}}, \"uid\":\"4ee490f5-9a8c-4f30-86f9-61dea799b972\"}, \"spec\":map[string]interface {}{\"env\":interface {}(nil), \"image\":\"\", \"repository\":\"mumoshu/actions-runner-controller-ci\"}}: validation failure list:\nspec.env in body must be of type array: \"null\""} github.com/go-logr/zapr.(*zapLogger).Error ``` and while the runnerdeployment controller is trying to create/update runners. I've fixed it so that the new `RunnerDeployment` example added to README just works.
mumoshu
force-pushed
the
manual-test-feedbacks
branch
from
b9c6f46 to
d12eca2
Compare
Collaborator
Author
|
@summerwind Thanks for your review. This should be good to be reviewed once again :) |
Contributor
|
LGTM 👍 Thank you! |
Merged
mumoshu
pushed a commit
that referenced
this pull request
Jan 24, 2021
mumoshu
pushed a commit
that referenced
this pull request
Jan 24, 2021
* Add chart workflows (#1) * Add chart workflows * Fix publishing step in CI Signed-off-by: David Young <davidy@funkypenguin.co.nz> * Update CI on push-to-master (#3) * Put helm installation step in the correct CI job Signed-off-by: David Young <davidy@funkypenguin.co.nz> * Put helm installation step in the correct CI job (#4) * Update on-push-master-publish-chart.yml * Remove references to certmanager dependency Signed-off-by: David Young <davidy@funkypenguin.co.nz> * Add ability to customize kube-rbac-proxy image Signed-off-by: David Young <davidy@funkypenguin.co.nz> * Only install cert-manager if we're going to spin up KinD Signed-off-by: David Young <davidy@funkypenguin.co.nz>
TingluoHuang
added a commit
that referenced
this pull request
Jan 12, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I'd like to add two fixes for the issues I've observed in manual testing. See each commit message for more details.
In nutshell,
RunnerDeploymentjust works as explained in README after these fixes!