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.

Sign up for GitHub

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

Jump to bottom

Chore: make operator clusterscoped #9549

Closed
wants to merge 1 commit into from
Closed

Chore: make operator clusterscoped #9549

wants to merge 1 commit into from

Conversation

alejandroEsc
Copy link
Contributor

First run on making redpanda tests, a few issues also found and fixed.

Backports Required

  • none - not a bug fix
  • none - this is a backport
  • none - issue does not exist in previous branches
  • none - papercut/not impactful enough to backport
  • v23.1.x
  • v22.3.x
  • v22.2.x

Release Notes

  • none

@alejandroEsc alejandroEsc self-assigned this Mar 18, 2023
@alejandroEsc alejandroEsc marked this pull request as ready for review March 18, 2023 01:46
@alejandroEsc alejandroEsc requested a review from a team as a code owner March 18, 2023 01:46
@alejandroEsc alejandroEsc changed the title Ae/issues/9106 Chore: add new k8s kuttl operator tests, Mar 20, 2023
alejandroEsc
alejandroEsc commented Mar 20, 2023
View reviewed changes
src/go/k8s/Makefile Outdated
@@ -6,7 +6,7 @@ CONFIGURATOR_IMG_LATEST ?= "localhost/configurator:dev"
CRD_OPTIONS ?= "crd:trivialVersions=true,preserveUnknownFields=false"

# default redpanda image to load
REDPANDA_IMG ?= "localhost/redpanda:dev"
REDPANDA_IMG ?= "localhost/redpand-operator:dev"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@RafalKorepta wanted to discuss this, as the make file makes little sense to me here

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, what is the question?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When you build it usually builds the redpanda-operator, but the kustomize is set up to use the repanda:dev image, which makes no sense, was this intentional or is this an error?

Copy link
Contributor

Choose a reason for hiding this comment

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

REDPANDA_IMG is a placeholder for Redpanda container not operator. This is used in kuttle tests everywhere. It should point to the tip of the current branch that test run on. It should be used in the kind load docker-image only.

I personally don't use Makefile to load image to my cluster.

I pull latest nightly build from docker hub and I'm running kuttle test locally after retagging it to redpanda:dev

@joejulian joejulian added the P1 High Priority - Sometime in the next 3 weeks label Mar 20, 2023
RafalKorepta
RafalKorepta reviewed Mar 23, 2023
View reviewed changes
src/go/k8s/tests/e2e/resource-redpanda-create/00-assert.yaml Outdated
Comment on lines 37 to 42
apiVersion: kuttl.dev/v1beta1
kind: TestAssert
collectors:
- type: pod
selector: app.kubernetes.io/name=redpanda
tail: -1
Copy link
Contributor

Choose a reason for hiding this comment

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

I personally would not use collectors as this pollutes stdout of the kuttle cli, but it's very hard to find the root cause. I would advise to do something like:

redpanda/src/go/k8s/tests/e2e/update-image-tls/00-assert.yaml

Lines 59 to 70 in 8a9d43f

collectors:
- command: /bin/bash -c "mkdir -p ../../_e2e_artifacts/update-image-tls/step-0"
- command: /bin/bash -c "kubectl -n $NAMESPACE get pods -o yaml > ../../_e2e_artifacts/update-image-tls/step-0/pods.yaml"
- command: /bin/bash -c "kubectl -n $NAMESPACE exec -c redpanda up-img-0 -- curl -sk https://up-img-0.up-img.$NAMESPACE.svc.cluster.local.:9644/v1/brokers > ../../_e2e_artifacts/update-image-tls/step-0/brokers-from-pod-0.json"
- command: /bin/bash -c "kubectl -n $NAMESPACE exec -c redpanda up-img-1 -- curl -sk https://up-img-1.up-img.$NAMESPACE.svc.cluster.local.:9644/v1/brokers > ../../_e2e_artifacts/update-image-tls/step-0/brokers-from-pod-1.json"
- command: /bin/bash -c "kubectl -n $NAMESPACE exec -c redpanda up-img-2 -- curl -sk https://up-img-2.up-img.$NAMESPACE.svc.cluster.local.:9644/v1/brokers > ../../_e2e_artifacts/update-image-tls/step-0/brokers-from-pod-2.json"
- command: /bin/bash -c "kubectl -n $NAMESPACE exec -c redpanda up-img-0 -- curl -sk https://up-img-0.up-img.$NAMESPACE.svc.cluster.local.:9644/v1/cluster_config/status > ../../_e2e_artifacts/update-image-tls/step-0/config-status-from-pod-0.json"
- command: /bin/bash -c "kubectl -n $NAMESPACE exec -c redpanda up-img-1 -- curl -sk https://up-img-1.up-img.$NAMESPACE.svc.cluster.local.:9644/v1/cluster_config/status > ../../_e2e_artifacts/update-image-tls/step-0/config-status-from-pod-1.json"
- command: /bin/bash -c "kubectl -n $NAMESPACE exec -c redpanda up-img-2 -- curl -sk https://up-img-2.up-img.$NAMESPACE.svc.cluster.local.:9644/v1/cluster_config/status > ../../_e2e_artifacts/update-image-tls/step-0/config-status-from-pod-2.json"
- command: /bin/bash -c "kubectl -n $NAMESPACE logs up-img-0 -c redpanda > ../../_e2e_artifacts/update-image-tls/step-0/logs-from-pod-0.txt"
- command: /bin/bash -c "kubectl -n $NAMESPACE logs up-img-1 -c redpanda > ../../_e2e_artifacts/update-image-tls/step-0/logs-from-pod-1.txt"
- command: /bin/bash -c "kubectl -n $NAMESPACE logs up-img-2 -c redpanda > ../../_e2e_artifacts/update-image-tls/step-0/logs-from-pod-2.txt"

I know it's painful to maintain the paths and names, but I didn't check the env vars during collectors. Maybe there is options to make collectors more generic.

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 see, thank you for that!

RafalKorepta
RafalKorepta approved these changes Mar 23, 2023
View reviewed changes
Copy link
Contributor

@RafalKorepta RafalKorepta left a comment

Choose a reason for hiding this comment

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

LGTM

@RafalKorepta
Copy link
Contributor

It appears that operator-v2 is missing in vtools. I will create it and re-run the CI.

@RafalKorepta
Copy link
Contributor

And no k8s tests was run. If you could do rebase/commit rephrase or any other change that will trigger buildkite again, thanks

@alejandroEsc
Copy link
Contributor Author

alejandroEsc commented Mar 24, 2023 •
edited

And no k8s tests was run. If you could do rebase/commit rephrase or any other change that will trigger buildkite again, thanks

Yes! I want to add a PR before this one and then rebase this one so that the Update tests are valid.

PR I would like to merge first: #9641

@alejandroEsc alejandroEsc force-pushed the ae/issues/9106 branch 2 times, most recently from 81d3230 to bcc7ac1 Compare March 24, 2023 19:59
@RafalKorepta
Copy link
Contributor

A lot lint issues
https://buildkite.com/redpanda/redpanda/builds/25924#01872891-f7d7-4273-a62b-7be630d485b8/305-390

@alejandroEsc alejandroEsc removed the P1 High Priority - Sometime in the next 3 weeks label Mar 28, 2023
@alejandroEsc
chore: add new tests for redpanda resource
7c30c42 
fix: finished testing additions
@alejandroEsc alejandroEsc changed the title Chore: add new k8s kuttl operator tests, Chore: make operator clusterscoped Mar 28, 2023
@alejandroEsc
Copy link
Contributor Author

Closing and will reopen from a fork

@alejandroEsc alejandroEsc deleted the ae/issues/9106 branch March 28, 2023 20:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RafalKorepta RafalKorepta RafalKorepta approved these changes

@joejulian joejulian Awaiting requested review from joejulian

Assignees

@alejandroEsc alejandroEsc

Labels
area/k8s k8s/tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@alejandroEsc @RafalKorepta @joejulian