-
Notifications
You must be signed in to change notification settings - Fork 66
🌱 Add test-operator with custom controller #2070
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
Changes from all commits
17df817
f837ca4
13a56d3
2396e27
8564214
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
name: testdata | ||
|
||
on: | ||
workflow_dispatch: | ||
pull_request: | ||
merge_group: | ||
push: | ||
branches: | ||
- main | ||
|
||
jobs: | ||
unit-test: | ||
runs-on: ubuntu-latest | ||
steps: | ||
- uses: actions/checkout@v4 | ||
|
||
- uses: actions/setup-go@v5 | ||
with: | ||
go-version-file: go.mod | ||
|
||
- name: Run test-operator unit tests | ||
run: | | ||
make test-test-operator |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -246,11 +246,24 @@ test-unit: $(SETUP_ENVTEST) envtest-k8s-bins #HELP Run the unit tests | |
$(UNIT_TEST_DIRS) \ | ||
-test.gocoverdir=$(COVERAGE_UNIT_DIR) | ||
|
||
|
||
.PHONY: test-test-operator | ||
test-test-operator: $(SETUP_ENVTEST) envtest-k8s-bins | ||
KUBEBUILDER_ASSETS="$(shell $(SETUP_ENVTEST) use -p path $(ENVTEST_VERSION) $(SETUP_ENVTEST_BIN_DIR_OVERRIDE))" \ | ||
CGO_ENABLED=1 go test -tags '$(GO_BUILD_TAGS)' -count=1 -race -short ./testdata/images/bundles/test-operator/... | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hey! 👋 I think we should mock the scenarios just like our users would experience them. This means:
Therefore, we can keep our samples well-maintained. Now, doing all this at once might be a bit much, so I’m totally fine starting with fixed samples, and then we can evolve toward full automation in follow-ups and discuss all properly. However: We need to change the file structure:We can’t organize it as
ProposalCould we change it here to be like:
This way:
Let me know what you think! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. By following my suggestion here for we have a real Operator that can be maintained via automation #2070 (comment), could we please use a name like memcached-operator instead? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I kinda get what you're trying to do, but I don't think that's scalable or maintainable given the necessary controller, webhook, and crd code that will be required by the test-operators (also between versions of it), which will either require patching or copying over generated code, which can be brittle, error prone, and create problems that are annoying to debug and fix - the only benefit being that we'll know when the tools no longer work for building the fixtures. The point of the test-operator isn't to exercise the steps involved in creating an operator and to make sure that works with OLMv1. We already have a test that exercises this developer flow. The point is to have a small, simple, test-operator which we own and can be used to exercise different features through out our e2e tests. I'd prefer not to call it memcached-operator since it isn't. What's wrong with test-operator? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The raised problem here is with the path that does not allow we have those samples well maintained, well documented and well generated in the long term, and the names does not reflect to the reality, could we
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't understand why they can't be well maintained. It seems more natural to group the versions under the operator than the operator under the versions...? |
||
|
||
TEST_OPERATOR_CONTROLLERS_HOME=./testdata/images/controllers | ||
TEST_OPERATOR_CONTROLLERS=v1.0.0 v2.0.0 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this missing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nah, this is just for building the controller binaries. The 1.3.0 version uses the 1.0.0 controller. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, I did not understood it as well because
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is related to the currently existing e2e tests. The v1.3.0 bundle in the PR substitutes the v2.0.0 bundle in main [this] commit. There's only a change in the included manifests (namely the configmap) between v1.0.0 and v1.3.0. I.e. v1.3.0 still uses the same controller as v1.0.0. This make target builds only the controllers. The bundle image itself is built in testdata/push - which is how it's currently done anyway. |
||
|
||
.PHONY: $(TEST_OPERATOR_CONTROLLERS) | ||
$(TEST_OPERATOR_CONTROLLERS): | ||
go build $(GO_BUILD_FLAGS) $(GO_BUILD_EXTRA_FLAGS) -tags '$(GO_BUILD_TAGS)' -ldflags '$(GO_BUILD_LDFLAGS)' -gcflags '$(GO_BUILD_GCFLAGS)' -asmflags '$(GO_BUILD_ASMFLAGS)' -o $(TEST_OPERATOR_CONTROLLERS_HOME)/test-operator/$@/manager ./testdata/images/bundles/test-operator/$@/cmd/main.go | ||
|
||
.PHONY: image-registry | ||
E2E_REGISTRY_IMAGE=localhost/e2e-test-registry:devel | ||
image-registry: export GOOS=linux | ||
image-registry: export GOARCH=amd64 | ||
image-registry: ## Build the testdata catalog used for e2e tests and push it to the image registry | ||
image-registry: $(TEST_OPERATOR_CONTROLLERS) ## Build the testdata catalog used for e2e tests and push it to the image registry | ||
go build $(GO_BUILD_FLAGS) $(GO_BUILD_EXTRA_FLAGS) -tags '$(GO_BUILD_TAGS)' -ldflags '$(GO_BUILD_LDFLAGS)' -gcflags '$(GO_BUILD_GCFLAGS)' -asmflags '$(GO_BUILD_ASMFLAGS)' -o ./testdata/push/bin/push ./testdata/push/push.go | ||
$(CONTAINER_RUNTIME) build -f ./testdata/Dockerfile -t $(E2E_REGISTRY_IMAGE) ./testdata | ||
$(CONTAINER_RUNTIME) save $(E2E_REGISTRY_IMAGE) | $(KIND) load image-archive /dev/stdin --name $(KIND_CLUSTER_NAME) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,2 @@ | ||
push/bin | ||
images/controllers |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
/* | ||
Copyright 2025. | ||
|
||
Licensed under the Apache License, Version 2.0 (the "License"); | ||
you may not use this file except in compliance with the License. | ||
You may obtain a copy of the License at | ||
|
||
http://www.apache.org/licenses/LICENSE-2.0 | ||
|
||
Unless required by applicable law or agreed to in writing, software | ||
distributed under the License is distributed on an "AS IS" BASIS, | ||
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
See the License for the specific language governing permissions and | ||
limitations under the License. | ||
*/ | ||
|
||
// Package v1 contains API Schema definitions for the testolm v1 API group. | ||
// +kubebuilder:object:generate=false | ||
// +groupName=testolm.operatorframework.io | ||
package v1 | ||
|
||
import ( | ||
"k8s.io/apimachinery/pkg/runtime/schema" | ||
"sigs.k8s.io/controller-runtime/pkg/scheme" | ||
) | ||
|
||
var ( | ||
// GroupVersion is group version used to register these objects. | ||
GroupVersion = schema.GroupVersion{Group: "testolm.operatorframework.io", Version: "v1"} | ||
|
||
// SchemeBuilder is used to add go types to the GroupVersionKind scheme. | ||
SchemeBuilder = &scheme.Builder{GroupVersion: GroupVersion} | ||
|
||
// AddToScheme adds the types in this group-version to the given scheme. | ||
AddToScheme = SchemeBuilder.AddToScheme | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
/* | ||
Copyright 2025. | ||
|
||
Licensed under the Apache License, Version 2.0 (the "License"); | ||
you may not use this file except in compliance with the License. | ||
You may obtain a copy of the License at | ||
|
||
http://www.apache.org/licenses/LICENSE-2.0 | ||
|
||
Unless required by applicable law or agreed to in writing, software | ||
distributed under the License is distributed on an "AS IS" BASIS, | ||
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
See the License for the specific language governing permissions and | ||
limitations under the License. | ||
*/ | ||
|
||
package v1 | ||
|
||
import ( | ||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
) | ||
|
||
// TestOperatorSpec defines the desired state of TestOperator. | ||
type TestOperatorSpec struct { | ||
// +optional | ||
Message string `json:"message,omitempty"` | ||
} | ||
|
||
// TestOperatorStatus defines the observed state of TestOperator. | ||
type TestOperatorStatus struct { | ||
Echo string `json:"echo,omitempty"` | ||
} | ||
|
||
// +kubebuilder:object:root=true | ||
// +kubebuilder:subresource:status | ||
|
||
// TestOperator is the Schema for the testoperators API. | ||
type TestOperator struct { | ||
metav1.TypeMeta `json:",inline"` | ||
metav1.ObjectMeta `json:"metadata,omitempty"` | ||
|
||
Spec TestOperatorSpec `json:"spec,omitempty"` | ||
Status TestOperatorStatus `json:"status,omitempty"` | ||
} | ||
|
||
// +kubebuilder:object:root=true | ||
|
||
// TestOperatorList contains a list of TestOperator. | ||
type TestOperatorList struct { | ||
metav1.TypeMeta `json:",inline"` | ||
metav1.ListMeta `json:"metadata,omitempty"` | ||
Items []TestOperator `json:"items"` | ||
} | ||
|
||
func init() { | ||
SchemeBuilder.Register(&TestOperator{}, &TestOperatorList{}) | ||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a function of the Operator/Mock sample itself.
It should not be part of the
operator-controller
Makefile at all — it belongs inside the sample.Placing it in the main Makefile goes against important separation-of-concerns principles.
It introduces coupling that will make these components harder to maintain over time.
Keeping this logic self-contained in the sample is key to preserving clarity and long-term maintainability.