Skip to content
This repository has been archived by the owner on Oct 22, 2024. It is now read-only.

update operator tools #1052

Merged
merged 2 commits into from
Dec 17, 2021
Merged

update operator tools #1052

merged 2 commits into from
Dec 17, 2021

Conversation

pohly
Copy link
Contributor

@pohly pohly commented Nov 17, 2021 •
edited
Loading

The newer versions don't fix any specific issue that we are having, but staying
closer to upstream is always good. For example, we can rule out that the
ancient operator-sdk caused the install problem with OLM 0.19.1.

@pohly pohly requested a review from avalluri November 17, 2021 14:11
pohly
pohly commented Nov 17, 2021
View reviewed changes
deploy/kustomize/operator/kustomization.yaml Outdated
apiVersion: v1
kind: ServiceAccount
apiVersion: apps/v1
kind: Deployment
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 ServiceAccount does not exist when kustomize generates deploy/kustomize/olm-bundle. The Deployment references the same namespace and always exists, therefore it gets used.

pohly
pohly commented Nov 17, 2021
View reviewed changes
operator/operator.make Outdated
CATALOG_DIR=$(REPO_ROOT)/deploy/olm-catalog
BUNDLE_DIR=$(REPO_ROOT)/deploy/olm-bundle/$(MAJOR_MINOR_PATCH_VERSION)
CRD_DIR=$(REPO_ROOT)/deploy/crd
CONTROLLER_GEN=_work/bin/controller-gen-$(CONTROLLER_GEN_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.

I don't see a reason for using absolute paths. Doing everything relative to where we run make is easier.

pohly
pohly commented Nov 17, 2021
View reviewed changes
operator/operator.make
cd $(REPO_ROOT) && $(CONTROLLER_GEN) crd:trivialVersions=true,crdVersions=v1 paths=./pkg/apis/... output:dir=$(CRD_DIR)/
version="$(shell $(CONTROLLER_GEN) --version |cut -f2 -d':')"; \
sed -i "1s/^/# This file was generated by controller-gen$$version via 'make operator-generate-crd'\n/" $(CRD_DIR)/*
$(CONTROLLER_GEN) crd paths=./pkg/apis/... output:dir=$(CRD_DIR)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

trivialVersions was not recognized as parameter anymore. Do you know what it did?

crdVersions=v1 should be the default now.

pohly
pohly commented Nov 17, 2021
View reviewed changes
operator/operator.make
rm -rf $(BUNDLE_DIR)
mkdir -p $(BUNDLE_DIR)
# Generate input YAML first. This might fail...
_work/kustomize build --load-restrictor LoadRestrictionsNone $(MANIFESTS_DIR) >$(BUNDLE_DIR)/manifests.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.

Directly piping the output caused me some head scratching because kustomize failed and I overlooked the one-line error message... Now make aborts directly.

pohly
pohly commented Nov 17, 2021
View reviewed changes
operator/operator.make
_work/kustomize build --load-restrictor LoadRestrictionsNone $(MANIFESTS_DIR) >$(BUNDLE_DIR)/manifests.yaml
# Specifying --kustomize-dir seems redundant because stdin already contains our ClusterServiceVersion,
# but without it the final ClusterServiceVersion in the bundle just has some automatically generated fields.
cat $(BUNDLE_DIR)/manifests.yaml | $< generate bundle --kustomize-dir=$(MANIFESTS_DIR) --version=$(MAJOR_MINOR_PATCH_VERSION) --output-dir=$(BUNDLE_DIR) --channels ${CHANNELS} --default-channel ${DEFAULT_CHANNEL}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

operator-sdk wants a pipe as input, not an open file.

pohly
pohly commented Nov 17, 2021
View reviewed changes
operator/operator.make
$(PATCH_VERSIONS) $(BUNDLE_DIR)/manifests/pmem-csi-operator.clusterserviceversion.yaml
ifdef REPLACES
@$(PATCH_REPLACES) $(BUNDLE_DIR)/manifests/pmem-csi-operator.clusterserviceversion.yaml
$(PATCH_REPLACES) $(BUNDLE_DIR)/manifests/pmem-csi-operator.clusterserviceversion.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.

Silently doing things made it harder to debug the make failure.

@pohly pohly mentioned this pull request Nov 17, 2021
Closed
@avalluri
Copy link
Contributor

make test fails:

[2021-11-17T14:31:49.502Z] hack/verify-generated.sh

[2021-11-17T14:31:49.502Z] + make generate

[2021-11-17T14:31:49.502Z] make[1]: Entering directory '/mnt/workspace/pmem-csi_PR-1052'

[2021-11-17T14:31:49.502Z] make[1]: Leaving directory '/mnt/workspace/pmem-csi_PR-1052'

[2021-11-17T14:31:49.502Z] make[1]: *** No rule to make target 'controller-gen', needed by 'operator-generate-k8s'.  Stop.

[2021-11-17T14:31:49.502Z] ERROR: code generation failed.

[2021-11-17T14:31:49.502Z] make: *** [test/test.make:33: test_generated] Error 1

@pohly
Copy link
Contributor Author

pohly commented Nov 18, 2021

make test fails

Fixed.

Somehow I can no longer reproduce the validation error about the service account, which makes me nervous. Something is fishy here, but I don't know what it is.

I've taken out the service account removal.

@pohly pohly force-pushed the olm-update branch 2 times, most recently from b7861db to a93c3e6 Compare November 25, 2021 12:12
@pohly pohly force-pushed the olm-update branch 2 times, most recently from c506520 to 230ce5d Compare December 15, 2021 11:37
@pohly
operator-sdk 1.15.0, kustomize 4.4.1, controller-gen 0.7.0
4e8e9dc 
The newer versions don't fix any specific issue that we are having, but staying
closer to upstream is always good. For example, we can rule out that the
ancient operator-sdk caused the install problem with OLM 0.19.1.
@pohly
e2e: test flake during metrics testing
2c0baac 
At least once the "direct-testing-metrics data full" test failed with:

Dec 15 22:36:03.221: Timed out after 10.005s.
Expected
    <string>: have pod IP
    Expected
        <string>:
    not to be empty
    GET failed
    Unexpected error:
        <*url.Error | 0xc000cb0e70>: {
            Op: "Get",
            URL: "http://default.format-node-hcz2c:10010/metrics",
            Err: <*errors.errorString | 0xc0015c9800>{
                s: "dialer failed: unable to upgrade connection: pod not found (\"format-node-hcz2c_default\")",
            },
        }
        Get "http://default.format-node-hcz2c:10010/metrics": dialer failed: unable to upgrade connection: pod not found ("format-node-hcz2c_default")
    occurred
to be empty

Better assertion annotations are necessary to understand which pod these
assertions are about.

If this was a pod that just began starting, then the 10s timeout may have been
too short.
@pohly pohly merged commit 70ecb2b into intel:devel Dec 17, 2021
pohly added a commit that referenced this pull request Jan 2, 2022
@pohly
e2e: fix OLM version skew testing
c86b761 
#1052 simplified the make rules
for the OLM bundle. However, that had the effect that those rules now
no longer can build the OLM bundle for the previous release, 1.0.1, which
caused version skew testing to fail.

The solution is to run make in the 1.0.1 source directory and thus use
the make rules from that release. This is cleaner, too.
@pohly pohly mentioned this pull request Jan 2, 2022
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@avalluri avalluri Awaiting requested review from avalluri

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@pohly @avalluri