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

added memoryLimit_field_under_registry_operator #1429 #100

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Horiodino
Copy link
Contributor

Description of Changes

added the memoryLimit field under registry_operator so that whenever the memory label is updated.

Related Issue(s)

devfile/api#1429

Acceptance Criteria

Tests

  • Test Coverage
    • Are your changes sufficiently tested, and are any applicable test cases added or updated to cover your changes?
  • Gosec scans

Documentation

  • Does the registry operator documentation need to be updated with your changes?

Tests Performed

Explain what tests you personally ran to ensure the changes are functioning as expected.

How To Test

Instructions for the reviewer on how to test your changes.

Running Unit Tests

Running Integration Tests

Notes To Reviewer

Any notes you would like to include for the reviewer.

Copy link

openshift-ci bot commented Sep 20, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Horiodino
Once this PR has been reviewed and has the lgtm label, please assign michael-valdron for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copy link

openshift-ci bot commented Sep 20, 2024

Hi @Horiodino. Thanks for your PR.

I'm waiting for a devfile member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Copy link
Contributor

@Jdubrick Jdubrick left a comment

Choose a reason for hiding this comment

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

Thanks for your PR @Horiodino. We recently implemented new PR templates. Would you happen to be able to provide any tests you ran to verify this is working as intended?

I need to look into whether or not our CI is failing due to the workflow itself or the k8s.io/apimachinery/pkg/api/resource import being weird but any local tests you performed would help as well.

Copy link
Contributor

@Jdubrick Jdubrick left a comment

Choose a reason for hiding this comment

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

In response to the Slack thread where you were experiencing issues in testing I found the following:

I built the operator image using your branch as reference, while trying to run make install && make deploy to deploy the operator I was met with a segfault that I included below. I was able to build and run the operator and tests with our main branch so that indicates to me that there is an issue (most likely with memory based on the changes) that is causing the fault.

I know you mentioned this was happening in the Slack thread even when you were on main, I believe that is happening because you probably had the IMG environment variable set to your broken image still, instead of the devfile main one.

Even if you can't run the integration tests without access to Openshift you should still be able to build and deploy the operator itself to verify it can be tested.

You can view my steps taken and results below for reference:

~/testing/praful-registry-operator-test/registry-operator (Implement_memoryLimit_field_under_registry_operator_reconcile-#1429 ✔) make podman-build
podman build . -t quay.io/devfile/registry-operator:next --build-arg TARGETARCH=amd64 \
--build-arg ENABLE_WEBHOOKS=true \
--build-arg ENABLE_WEBHOOK_HTTP2=false
[1/2] STEP 1/15: FROM golang:1.21@sha256:b405b620c7b53ef64695c7da7c8396f411f381c1eb7da6713c585dd7eca1559b AS builder
Resolving "golang" using unqualified-search registries (/etc/containers/registries.conf.d/999-podman-machine.conf)
Trying to pull docker.io/library/golang@sha256:b405b620c7b53ef64695c7da7c8396f411f381c1eb7da6713c585dd7eca1559b...
Getting image source signatures
Copying blob sha256:f25e25a1338cd9a74320d3614da987b4eee0d8fb2408a8d4ffdfe3322d412fd4
Copying blob sha256:51cd5f8f608f832afd85dc82fbac4aea05183fd7fccf555dd4a53a4bbe06b013
Copying blob sha256:1e60a453843e00d6f3d4242dbd696365f0894e3ca2f02f4ce1ab098d7ff7907f
Copying blob sha256:4dadad3edfd860d6d4fd52d4cbf17e7431a88d64161c62654786e60f331343a8
Copying blob sha256:a8df69f15c51d1fa8f80fac8aa3783284f02e1cb10fb38d294be12a39a814e30
Copying blob sha256:8bf1f3ab454abe873b1a5ce2cd1c82cf92990da3831b93035475749cb99e15e8
Copying blob sha256:4f4fb700ef54461cfa02571ae0db9a0dc1e0cdb5577484a6d75e68dc38e8acc1
Copying config sha256:ee55db4bbb11b34c961dc0ee45e08eed865d8dace8dba8c5b804ccf8c0a9bdc1
Writing manifest to image destination
[1/2] STEP 2/15: ARG TARGETARCH=amd64
--> 687e2a850d48
[1/2] STEP 3/15: WORKDIR /workspace
--> e381df645b41
[1/2] STEP 4/15: COPY go.mod go.mod
--> 177486206402
[1/2] STEP 5/15: COPY go.sum go.sum
--> fd11f4a6ec3f
[1/2] STEP 6/15: RUN go mod download
--> 2f17daa80ec5
[1/2] STEP 7/15: COPY main.go main.go
--> 5b2bd2bb6e9e
[1/2] STEP 8/15: COPY api/ api/
--> a46435eae84c
[1/2] STEP 9/15: COPY controllers/ controllers/
--> ea57c54fb929
[1/2] STEP 10/15: COPY pkg/ pkg/
--> 9d777e5b658d
[1/2] STEP 11/15: RUN CGO_ENABLED=0 GOOS=linux GOARCH=${TARGETARCH} GO111MODULE=on go build -a -o manager main.go
--> 76651a75de7f
[1/2] STEP 12/15: ARG ENABLE_WEBHOOKS=true
--> bbeb842e9ba9
[1/2] STEP 13/15: ENV ENABLE_WEBHOOKS=${ENABLE_WEBHOOKS}
--> c884dfe34bab
[1/2] STEP 14/15: ARG ENABLE_WEBHOOK_HTTP2=false
--> 2fe8b87c991c
[1/2] STEP 15/15: ENV ENABLE_WEBHOOK_HTTP2=${ENABLE_WEBHOOK_HTTP2}
--> 68958de14a46
[2/2] STEP 1/5: FROM gcr.io/distroless/static:nonroot-arm64
Trying to pull gcr.io/distroless/static:nonroot-arm64...
Getting image source signatures
Copying blob sha256:33e068de264953dfdc9f9ada207e76b61159721fd64a4820b320d05133a55fb8
Copying blob sha256:04859ab82af68c4114de59e434203d6b5260ed7c00f07f8176c8cc660337c26d
Copying blob sha256:8ffb3c3cf71ab16787d74e41347deae1495b9309bae0f0f542d4c5464c245489
Copying blob sha256:bfb59b82a9b65e47d485e53b3e815bca3b3e21a095bd0cb88ced9ac0b48062bf
Copying blob sha256:a62778643d563b511190663ef9a77c30d46d282facfdce4f3a7aecc03423c1f3
Copying blob sha256:7c12895b777bcaa8ccae0605b4de635b68fc32d60fa08f421dc3818bf55ee212
Copying blob sha256:5664b15f108bf9436ce3312090a767300800edbbfd4511aa1a6d64357024d5dd
Copying blob sha256:0bab15eea81d0fe6ab56ebf5fba14e02c4c1775a7f7436fbddd3505add4e18fa
Copying blob sha256:4aa0ea1413d37a58615488592a0b827ea4b2e48fa5a77cf707d0e35f025e613f
Copying blob sha256:da7816fa955ea24533c388143c78804c28682eef99b4ee3723b548c70148bba6
Copying blob sha256:9aee425378d2c16cd44177dc54a274b312897f5860a8e78fdfda555a0d79dd71
Copying config sha256:d2ac5a6821eae6d31320770e445892e8c4879f8dab479a23e2a4263fc495c45b
Writing manifest to image destination
[2/2] STEP 2/5: WORKDIR /
--> 092bc6395d62
[2/2] STEP 3/5: COPY --from=builder /workspace/manager .
--> 27f6634cd0bf
[2/2] STEP 4/5: USER 1001
--> c29039d101f9
[2/2] STEP 5/5: ENTRYPOINT ["/manager"]
[2/2] COMMIT quay.io/devfile/registry-operator:next
--> 72f8a5d6393b
Successfully tagged quay.io/devfile/registry-operator:next
72f8a5d6393b8d3af60ed3b6301c9bb1db23820d8cd504d6368d62a5cda4462a
~/testing/praful-registry-operator-test/registry-operator (Implement_memoryLimit_field_under_registry_operator_reconcile-#1429 ✔) podman tag quay.io/devfile/registry-operator:next quay.io/rh-ee-jdubrick/registry-opera
tor:test-pr-praful
~/testing/praful-registry-operator-test/registry-operator (Implement_memoryLimit_field_under_registry_operator_reconcile-#1429 ✔) podman push quay.io/rh-ee-jdubrick/registry-operator:test-pr-praful
Getting image source signatures
Copying blob sha256:ac805962e47900b616b2f4b4584a34ac7b07d64ac1fd2c077478cf65311addcc
Copying blob sha256:3621a240af3b62b2a2d63f19493da2952ae7f5cc28a961dce68931945eb7676c
Copying blob sha256:ddc6e550070ca022d94bd4415de20545ba69954033b4985045a8b05f538bbe5c
Copying blob sha256:8fa10c0194df9b7c054c90dbe482585f768a54428fc90a5b78a0066a123b1bba
Copying blob sha256:4d049f83d9cf21d1f5cc0e11deaf36df02790d0e60c1a3829538fb4b61685368
Copying blob sha256:af5aa97ebe6ce1604747ec1e21af7136ded391bcabe4acef882e718a87c86bcc
Copying blob sha256:bbb6cacb8c82e4da4e8143e03351e939eab5e21ce0ef333c42e637af86c5217b
Copying blob sha256:2a92d6ac9e4fcc274d5168b217ca4458a9fec6f094ead68d99c77073f08caac1
Copying blob sha256:1a73b54f556b477f0a8b939d13c504a3b4f4db71f7a09c63afbc10acb3de5849
Copying blob sha256:f4aee9e53c42a22ed82451218c3ea03d1eea8d6ca8fbe8eb4e950304ba8a8bb3
Copying blob sha256:b336e209998fa5cf0eec3dabf93a21194198a35f4f75612d8da03693f8c30217
Copying blob sha256:01e02552735537c1c6947f67f5674b4c36d3f8bec8c668684007732280315190
Copying config sha256:72f8a5d6393b8d3af60ed3b6301c9bb1db23820d8cd504d6368d62a5cda4462a
Writing manifest to image destination
~/testing/praful-registry-operator-test/registry-operator (Implement_memoryLimit_field_under_registry_operator_reconcile-#1429 ✔) make install-cert
kubectl apply -f https://github.com/cert-manager/cert-manager/releases/download/v1.15.1/cert-manager.yaml
namespace/cert-manager created
customresourcedefinition.apiextensions.k8s.io/certificaterequests.cert-manager.io created
customresourcedefinition.apiextensions.k8s.io/certificates.cert-manager.io created
customresourcedefinition.apiextensions.k8s.io/challenges.acme.cert-manager.io created
customresourcedefinition.apiextensions.k8s.io/clusterissuers.cert-manager.io created
customresourcedefinition.apiextensions.k8s.io/issuers.cert-manager.io created
customresourcedefinition.apiextensions.k8s.io/orders.acme.cert-manager.io created
serviceaccount/cert-manager-cainjector created
serviceaccount/cert-manager created
serviceaccount/cert-manager-webhook created
clusterrole.rbac.authorization.k8s.io/cert-manager-cainjector created
clusterrole.rbac.authorization.k8s.io/cert-manager-controller-issuers created
clusterrole.rbac.authorization.k8s.io/cert-manager-controller-clusterissuers created
clusterrole.rbac.authorization.k8s.io/cert-manager-controller-certificates created
clusterrole.rbac.authorization.k8s.io/cert-manager-controller-orders created
clusterrole.rbac.authorization.k8s.io/cert-manager-controller-challenges created
clusterrole.rbac.authorization.k8s.io/cert-manager-controller-ingress-shim created
clusterrole.rbac.authorization.k8s.io/cert-manager-cluster-view created
clusterrole.rbac.authorization.k8s.io/cert-manager-view created
clusterrole.rbac.authorization.k8s.io/cert-manager-edit created
clusterrole.rbac.authorization.k8s.io/cert-manager-controller-approve:cert-manager-io created
clusterrole.rbac.authorization.k8s.io/cert-manager-controller-certificatesigningrequests created
clusterrole.rbac.authorization.k8s.io/cert-manager-webhook:subjectaccessreviews created
clusterrolebinding.rbac.authorization.k8s.io/cert-manager-cainjector created
clusterrolebinding.rbac.authorization.k8s.io/cert-manager-controller-issuers created
clusterrolebinding.rbac.authorization.k8s.io/cert-manager-controller-clusterissuers created
clusterrolebinding.rbac.authorization.k8s.io/cert-manager-controller-certificates created
clusterrolebinding.rbac.authorization.k8s.io/cert-manager-controller-orders created
clusterrolebinding.rbac.authorization.k8s.io/cert-manager-controller-challenges created
clusterrolebinding.rbac.authorization.k8s.io/cert-manager-controller-ingress-shim created
clusterrolebinding.rbac.authorization.k8s.io/cert-manager-controller-approve:cert-manager-io created
clusterrolebinding.rbac.authorization.k8s.io/cert-manager-controller-certificatesigningrequests created
clusterrolebinding.rbac.authorization.k8s.io/cert-manager-webhook:subjectaccessreviews created
role.rbac.authorization.k8s.io/cert-manager-cainjector:leaderelection created
role.rbac.authorization.k8s.io/cert-manager:leaderelection created
role.rbac.authorization.k8s.io/cert-manager-webhook:dynamic-serving created
rolebinding.rbac.authorization.k8s.io/cert-manager-cainjector:leaderelection created
rolebinding.rbac.authorization.k8s.io/cert-manager:leaderelection created
rolebinding.rbac.authorization.k8s.io/cert-manager-webhook:dynamic-serving created
service/cert-manager created
service/cert-manager-webhook created
deployment.apps/cert-manager-cainjector created
deployment.apps/cert-manager created
deployment.apps/cert-manager-webhook created
mutatingwebhookconfiguration.admissionregistration.k8s.io/cert-manager-webhook created
validatingwebhookconfiguration.admissionregistration.k8s.io/cert-manager-webhook created
~/testing/praful-registry-operator-test/registry-operator (Implement_memoryLimit_field_under_registry_operator_reconcile-#1429 ✔) export IMG=quay.io/rh-ee-jdubrick/regisry-operator:test-pr-praful
~/testing/praful-registry-operator-test/registry-operator (Implement_memoryLimit_field_under_registry_operator_reconcile-#1429 ✔) make install && make deploy
mkdir -p /Users/jordandubrick/testing/praful-registry-operator-test/registry-operator/bin
test -s /Users/jordandubrick/testing/praful-registry-operator-test/registry-operator/bin/controller-gen && /Users/jordandubrick/testing/praful-registry-operator-test/registry-operator/bin/controller-gen --version | grep -q v0.9.2 || \
GOBIN=/Users/jordandubrick/testing/praful-registry-operator-test/registry-operator/bin GOFLAGS="" go install sigs.k8s.io/controller-tools/cmd/[email protected]
/Users/jordandubrick/testing/praful-registry-operator-test/registry-operator/bin/controller-gen rbac:roleName=manager-role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
        panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x0 pc=0x1030fcbf4]

goroutine 162 [running]:
go/types.(*Checker).handleBailout(0x140003a8c00, 0x14001ef3d18)
        /opt/homebrew/Cellar/go/1.22.5/libexec/src/go/types/check.go:367 +0x9c
panic({0x1033c7a80?, 0x103957f50?})
        /opt/homebrew/Cellar/go/1.22.5/libexec/src/runtime/panic.go:770 +0x124
go/types.(*StdSizes).Sizeof(0x0, {0x103490cb0, 0x103960800})
        /opt/homebrew/Cellar/go/1.22.5/libexec/src/go/types/sizes.go:228 +0x314
go/types.(*Config).sizeof(...)
        /opt/homebrew/Cellar/go/1.22.5/libexec/src/go/types/sizes.go:333
go/types.representableConst.func1({0x103490cb0?, 0x103960800?})
        /opt/homebrew/Cellar/go/1.22.5/libexec/src/go/types/const.go:76 +0x9c
go/types.representableConst({0x103496e98, 0x10392c860}, 0x140003a8c00, 0x103960800, 0x14001ef3468)
        /opt/homebrew/Cellar/go/1.22.5/libexec/src/go/types/const.go:92 +0x138
go/types.(*Checker).representation(0x140003a8c00, 0x14001b2b240, 0x103960800)
        /opt/homebrew/Cellar/go/1.22.5/libexec/src/go/types/const.go:256 +0x68
go/types.(*Checker).implicitTypeAndValue(0x140003a8c00, 0x14001b2b240, {0x103490cd8, 0x140002e65b0})
        /opt/homebrew/Cellar/go/1.22.5/libexec/src/go/types/expr.go:375 +0x304
go/types.(*Checker).assignment(0x140003a8c00, 0x14001b2b240, {0x103490cd8, 0x140002e65b0}, {0x10320ecab, 0x14})
        /opt/homebrew/Cellar/go/1.22.5/libexec/src/go/types/assignments.go:52 +0x23c
go/types.(*Checker).initConst(0x140003a8c00, 0x1400198c4e0, 0x14001b2b240)
        /opt/homebrew/Cellar/go/1.22.5/libexec/src/go/types/assignments.go:126 +0x274
go/types.(*Checker).constDecl(0x140003a8c00, 0x1400198c4e0, {0x103493830, 0x14001fa65a0}, {0x103493830, 0x14001fa65c0}, 0x0)
        /opt/homebrew/Cellar/go/1.22.5/libexec/src/go/types/decl.go:490 +0x250
go/types.(*Checker).objDecl(0x140003a8c00, {0x10349bf20, 0x1400198c4e0}, 0x0)
        /opt/homebrew/Cellar/go/1.22.5/libexec/src/go/types/decl.go:191 +0x84c
go/types.(*Checker).packageObjects(0x140003a8c00)
        /opt/homebrew/Cellar/go/1.22.5/libexec/src/go/types/resolver.go:693 +0x468
go/types.(*Checker).checkFiles(0x140003a8c00, {0x14001f8e030, 0x5, 0x5})
        /opt/homebrew/Cellar/go/1.22.5/libexec/src/go/types/check.go:408 +0x164
go/types.(*Checker).Files(...)
        /opt/homebrew/Cellar/go/1.22.5/libexec/src/go/types/check.go:372
sigs.k8s.io/controller-tools/pkg/loader.(*loader).typeCheck(0x14000319140, 0x140018cdb80)
        /Users/jordandubrick/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/loader/loader.go:286 +0x2d8
sigs.k8s.io/controller-tools/pkg/loader.(*Package).NeedTypesInfo(0x140018cdb80)
        /Users/jordandubrick/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/loader/loader.go:99 +0x44
sigs.k8s.io/controller-tools/pkg/loader.(*TypeChecker).check(0x14001263f50, 0x140018cdb80)
        /Users/jordandubrick/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/loader/refs.go:268 +0x304
sigs.k8s.io/controller-tools/pkg/loader.(*TypeChecker).check.func1(0x6e6f632f646c6975?)
        /Users/jordandubrick/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/loader/refs.go:262 +0x58
created by sigs.k8s.io/controller-tools/pkg/loader.(*TypeChecker).check in goroutine 111
        /Users/jordandubrick/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/loader/refs.go:260 +0x230
make: *** [manifests] Error 2

@thepetk
Copy link
Contributor

thepetk commented Oct 3, 2024

/retest

I see the ci/prow/v4.15-registry-operator-integration-test job has failed for a non-related reason.

@Jdubrick
Copy link
Contributor

Jdubrick commented Oct 3, 2024

/retest

Copy link

openshift-ci bot commented Oct 3, 2024

@Horiodino: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/v4.15-registry-operator-integration-test e3b8a9c link true /test v4.15-registry-operator-integration-test

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants