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

oadp-1.3: Make deploy-olm work on 4.16 and golang 1.22 built-ins #1441

Closed

Conversation

kaovilai
Copy link
Member

@kaovilai kaovilai commented Jun 22, 2024

  • make deploy-olm for 4.16 adding --security-context-config restricted
  • Upgrade controller-gen so make manifests runs

Why the changes were made

kubernetes-sigs/controller-tools#945

How to test the changes made

@kaovilai kaovilai changed the title Make deploy-olm work on 4.16 and golang 1.22 built-ins oadp-1.3: Make deploy-olm work on 4.16 and golang 1.22 built-ins Jun 22, 2024
Copy link

openshift-ci bot commented Jun 22, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kaovilai

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

The pull request process is described 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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 22, 2024
@kaovilai kaovilai force-pushed the controller-gen-v0.14.0-oadp-1.3 branch 2 times, most recently from db67a9f to 552350a Compare June 22, 2024 21:09
@mateusoliveira43
Copy link
Contributor

mateusoliveira43 commented Jun 23, 2024

Do we want these changes in oadp-1.3? Since its Go 1.20 (ref https://github.com/openshift/oadp-operator/blob/oadp-1.3/go.mod#L3) this is not needed, but it makes development easier (but as discussed, oadp-1.3 should not work with 4.16, right?)

If there is an easy way to install multiple Go versions in system, would try this way (in Python world, there is a tool that lets you install a specific Python version on a folder only, for example)

What I have been doing to work on old Go versions is to use containers. Example:

Create container image from this Dockerfile

FROM golang:1.20

ARG USER_NAME=dev

RUN useradd --create-home "$USER_NAME"

USER $USER_NAME

WORKDIR /home/$USER_NAME/repo
docker image build --tag oadp-development -f dev.Dockerfile .

Then, connect to container shell

docker container run \
  -u $(id -u):$(id -g) -h container \
  -v $PWD:/home/dev/repo \
  -it --rm oadp-development bash

and run Go command inside it

@kaovilai
Copy link
Member Author

If there is an easy way to install multiple Go versions in system, would try this way (in Python world, there is a tool that lets you install a specific Python version on a folder only, for example)

Pre golang 1.21: https://github.com/go-simpler/goversion
Post golang 1.21: GOTOOLCHAIN=go1.18 go version

However GOTOOLCHAIN still does not download older go .../libexec/ builtins libs, thus it's not a complete replacement of goversion

Copy link
Member Author

@kaovilai kaovilai left a comment

Choose a reason for hiding this comment

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

controller-gen bump can be removed if you use golang 1.20.

operator-sdk bump is mandatory if we want to use

$(OPERATOR_SDK) run bundle $(THIS_BUNDLE_IMAGE) --namespace $(OADP_TEST_NAMESPACE) --security-context-config restricted

@kaovilai kaovilai force-pushed the controller-gen-v0.14.0-oadp-1.3 branch from 552350a to 7999d99 Compare June 25, 2024 18:22
@kaovilai
Copy link
Member Author

mongo app needs to remove restartPolicy as well from its deploy template spec on non-init containers.

error creating or updating resource: apps/v1, Kind=Deployment; name: mysql; error: Deployment.apps "mysql" is invalid: spec.template.spec.containers[0].restartPolicy: Forbidden: may not be set for non-init containers; retrying for 1 more times

@kaovilai
Copy link
Member Author

restart policy addressed in: #1437

@sseago
Copy link
Contributor

sseago commented Jun 26, 2024

I thought 4.16 wasn't supported on oadp-1.3? Do we need 4.16 changes here?

@kaovilai
Copy link
Member Author

Well I don't have a cluster older than 4.16 to test on :/

@@ -328,6 +328,8 @@ bundle: manifests kustomize operator-sdk ## Generate bundle manifests and metada
# TODO: update CI to use generated one
cp bundle.Dockerfile build/Dockerfile.bundle
GOFLAGS="-mod=mod" $(OPERATOR_SDK) bundle validate ./bundle
sed -e 's/ createdAt: .*/$(shell grep -I '^ createdAt: ' bundle/manifests/oadp-operator.clusterserviceversion.yaml)/' bundle/manifests/oadp-operator.clusterserviceversion.yaml > bundle/manifests/oadp-operator.clusterserviceversion.yaml.tmp
Copy link
Contributor

Choose a reason for hiding this comment

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

You could use sed -i[SUFFIX] or sed --in-place=[SUFFIX] which will create backup for you.

Copy link
Member Author

Choose a reason for hiding this comment

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

ICYMI sed --in-place doesn't work well on macos ;P

Copy link
Member Author

Choose a reason for hiding this comment

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

Also per https://stackoverflow.com/questions/7573368/in-place-edits-with-sed-on-os-x

The Unix Way™ would (IMHO) be to use sed non-destructively, test that it exited cleanly, and only then remove the extraneous file.

Copy link
Member Author

Choose a reason for hiding this comment

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

oh yeah.. with suffix should work. I guess that's shorter.

@kaovilai kaovilai force-pushed the controller-gen-v0.14.0-oadp-1.3 branch from 7999d99 to ff2c8ab Compare October 1, 2024 01:44
Updates operator-sdk to versions that can `run bundle` with restricted security-context-config

Update controller-gen to v0.14.0 to avoid panic on newer golang and CRD_OPTIONS that works with it.

panic log
```
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=0x102676d94]

goroutine 23 [running]:
go/types.(*Checker).handleBailout(0x140008ec400, 0x1400323fd18)
        /opt/homebrew/Cellar/go/1.22.3/libexec/src/go/types/check.go:367 +0x9c
panic({0x102936800?, 0x102eb3ce0?})
        /opt/homebrew/Cellar/go/1.22.3/libexec/src/runtime/panic.go:770 +0x124
go/types.(*StdSizes).Sizeof(0x0, {0x1029fdb80, 0x102ebc5e0})
        /opt/homebrew/Cellar/go/1.22.3/libexec/src/go/types/sizes.go:228 +0x314
go/types.(*Config).sizeof(...)
        /opt/homebrew/Cellar/go/1.22.3/libexec/src/go/types/sizes.go:333
go/types.representableConst.func1({0x1029fdb80?, 0x102ebc5e0?})
        /opt/homebrew/Cellar/go/1.22.3/libexec/src/go/types/const.go:76 +0x9c
go/types.representableConst({0x102a03ec0, 0x102e88720}, 0x140008ec400, 0x102ebc5e0, 0x1400323ce78)
        /opt/homebrew/Cellar/go/1.22.3/libexec/src/go/types/const.go:106 +0x2b0
go/types.(*Checker).representation(0x140008ec400, 0x140013b5800, 0x102ebc5e0)
        /opt/homebrew/Cellar/go/1.22.3/libexec/src/go/types/const.go:256 +0x68
go/types.(*Checker).representable(0x140008ec400, 0x140013b5800, 0x102ebc5e0)
        /opt/homebrew/Cellar/go/1.22.3/libexec/src/go/types/const.go:239 +0x28
go/types.(*Checker).shift(0x140008ec400, 0x140013b57c0, 0x140013b5800, {0x102a01ca8, 0x140010360f0}, 0x14)
        /opt/homebrew/Cellar/go/1.22.3/libexec/src/go/types/expr.go:650 +0x1d8
go/types.(*Checker).binary(0x140008ec400, 0x140013b57c0, {0x102a01ca8, 0x140010360f0}, {0x102a021b8, 0x14001026ba0}, {0x102a021b8, 0x14001026bc0}, 0x14, 0xa6c0)
        /opt/homebrew/Cellar/go/1.22.3/libexec/src/go/types/expr.go:796 +0x100
go/types.(*Checker).exprInternal(0x140008ec400, 0x0, 0x140013b57c0, {0x102a01ca8, 0x140010360f0}, {0x0, 0x0})
        /opt/homebrew/Cellar/go/1.22.3/libexec/src/go/types/expr.go:1416 +0x1d4
go/types.(*Checker).rawExpr(0x140008ec400, 0x0, 0x140013b57c0, {0x102a01ca8?, 0x140010360f0?}, {0x0?, 0x0?}, 0x0)
        /opt/homebrew/Cellar/go/1.22.3/libexec/src/go/types/expr.go:979 +0x12c
go/types.(*Checker).expr(0x140008ec400, 0x102114d7c?, 0x140013b57c0, {0x102a01ca8?, 0x140010360f0?})
        /opt/homebrew/Cellar/go/1.22.3/libexec/src/go/types/expr.go:1513 +0x38
go/types.(*Checker).constDecl(0x140008ec400, 0x140013d2d80, {0x0, 0x0}, {0x102a01ca8, 0x140010360f0}, 0x0)
        /opt/homebrew/Cellar/go/1.22.3/libexec/src/go/types/decl.go:488 +0x23c
go/types.(*Checker).objDecl(0x140008ec400, {0x102a08e40, 0x140013d2d80}, 0x0)
        /opt/homebrew/Cellar/go/1.22.3/libexec/src/go/types/decl.go:191 +0x84c
go/types.(*Checker).ident(0x140008ec400, 0x140013b5780, 0x14001026620, 0x0, 0x0)
        /opt/homebrew/Cellar/go/1.22.3/libexec/src/go/types/typexpr.go:62 +0x1f0
go/types.(*Checker).exprInternal(0x140008ec400, 0x0, 0x140013b5780, {0x102a00808, 0x14001026620}, {0x0, 0x0})
        /opt/homebrew/Cellar/go/1.22.3/libexec/src/go/types/expr.go:1033 +0x114
go/types.(*Checker).rawExpr(0x140008ec400, 0x0, 0x140013b5780, {0x102a00808?, 0x14001026620?}, {0x0?, 0x0?}, 0x0)
        /opt/homebrew/Cellar/go/1.22.3/libexec/src/go/types/expr.go:979 +0x12c
go/types.(*Checker).expr(0x140008ec400, 0x2200010342430a?, 0x140013b5780, {0x102a00808?, 0x14001026620?})
        /opt/homebrew/Cellar/go/1.22.3/libexec/src/go/types/expr.go:1513 +0x38
go/types.(*Checker).binary(0x140008ec400, 0x140013b5740, {0x102a01ca8, 0x14001036030}, {0x102a00808, 0x14001026600}, {0x102a00808, 0x14001026620}, 0xc, 0x9e41)
        /opt/homebrew/Cellar/go/1.22.3/libexec/src/go/types/expr.go:784 +0x88
go/types.(*Checker).exprInternal(0x140008ec400, 0x0, 0x140013b5740, {0x102a01ca8, 0x14001036030}, {0x0, 0x0})
        /opt/homebrew/Cellar/go/1.22.3/libexec/src/go/types/expr.go:1416 +0x1d4
go/types.(*Checker).rawExpr(0x140008ec400, 0x0, 0x140013b5740, {0x102a01ca8?, 0x14001036030?}, {0x0?, 0x0?}, 0x0)
        /opt/homebrew/Cellar/go/1.22.3/libexec/src/go/types/expr.go:979 +0x12c
go/types.(*Checker).expr(0x140008ec400, 0x140013a3200?, 0x140013b5740, {0x102a01ca8?, 0x14001036030?})
        /opt/homebrew/Cellar/go/1.22.3/libexec/src/go/types/expr.go:1513 +0x38
go/types.(*Checker).constDecl(0x140008ec400, 0x140013a32c0, {0x0, 0x0}, {0x102a01ca8, 0x14001036030}, 0x0)
        /opt/homebrew/Cellar/go/1.22.3/libexec/src/go/types/decl.go:488 +0x23c
go/types.(*Checker).objDecl(0x140008ec400, {0x102a08e40, 0x140013a32c0}, 0x0)
        /opt/homebrew/Cellar/go/1.22.3/libexec/src/go/types/decl.go:191 +0x84c
go/types.(*Checker).packageObjects(0x140008ec400)
        /opt/homebrew/Cellar/go/1.22.3/libexec/src/go/types/resolver.go:693 +0x468
go/types.(*Checker).checkFiles(0x140008ec400, {0x14000191540, 0xa, 0xa})
        /opt/homebrew/Cellar/go/1.22.3/libexec/src/go/types/check.go:408 +0x164
go/types.(*Checker).Files(...)
        /opt/homebrew/Cellar/go/1.22.3/libexec/src/go/types/check.go:372
sigs.k8s.io/controller-tools/pkg/loader.(*loader).typeCheck(0x140002c8fc0, 0x14002f09c40)
        /Users/tiger/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/loader/loader.go:283 +0x2d8
sigs.k8s.io/controller-tools/pkg/loader.(*Package).NeedTypesInfo(0x14002f09c40)
        /Users/tiger/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/loader/loader.go:96 +0x44
sigs.k8s.io/controller-tools/pkg/loader.(*TypeChecker).check(0x14000fb1110, 0x14002f09c40)
        /Users/tiger/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/loader/refs.go:263 +0x304
sigs.k8s.io/controller-tools/pkg/loader.(*TypeChecker).check.func1(0x0?)
        /Users/tiger/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/loader/refs.go:257 +0x58
created by sigs.k8s.io/controller-tools/pkg/loader.(*TypeChecker).check in goroutine 1
        /Users/tiger/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/loader/refs.go:255 +0x230
make[1]: *** [manifests] Error 2
```

Signed-off-by: Tiger Kaovilai <[email protected]>

Upgrade controller-gen so `make manifests` runs

Signed-off-by: Tiger Kaovilai <[email protected]>
@kaovilai kaovilai force-pushed the controller-gen-v0.14.0-oadp-1.3 branch from ff2c8ab to 382a45a Compare October 1, 2024 01:44
Signed-off-by: Mateus Oliveira <[email protected]>
Signed-off-by: Tiger Kaovilai <[email protected]>
Dockerfile Outdated
Copy link
Member Author

Choose a reason for hiding this comment

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

Contains cherry-pick 139f4f5 from #1478 to deploy and test on 4.16 ARM courtesy of Mr. Green Helmet.

Signed-off-by: Tiger Kaovilai <[email protected]>
Copy link

openshift-ci bot commented Oct 1, 2024

@kaovilai: all tests passed!

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.

@mateusoliveira43
Copy link
Contributor

can we discuss this in scrum? I would try another approach for this #1441 (comment)

Ans also, keeping production requirements #1441 (comment)

@kaovilai
Copy link
Member Author

kaovilai commented Oct 1, 2024

adding to scrum

@kaovilai
Copy link
Member Author

kaovilai commented Oct 1, 2024

Scrum concluded that this is not going forward.. block 4.16 deploy-olm install on oadp-1.3 via makefile.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants