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

dont clear tenant if empty #1630

Closed
wants to merge 1 commit into from
Closed

dont clear tenant if empty #1630

wants to merge 1 commit into from

Conversation

cniackz
Copy link
Contributor

@cniackz cniackz commented Jun 8, 2023

Objective:

To solve Panic:

E0608 13:16:17.960769       1 runtime.go:79] Observed a panic: "invalid memory address or nil pointer dereference" (runtime error: invalid memory address or nil pointer dereference)
goroutine 184 [running]:
k8s.io/apimachinery/pkg/util/runtime.logPanic({0x1ca9e80?, 0x4da68b0})
	k8s.io/apimachinery@v0.26.1/pkg/util/runtime/runtime.go:75 +0x84
k8s.io/apimachinery/pkg/util/runtime.HandleCrash({0x0, 0x0, 0x4000716a80?})
	k8s.io/apimachinery@v0.26.1/pkg/util/runtime/runtime.go:49 +0x80
panic({0x1ca9e80, 0x4da68b0})
	runtime/panic.go:884 +0x20c
github.com/minio/operator/pkg/apis/minio.min.io/v2.(*Tenant).EnsureDefaults(0x4000540140?)
	github.com/minio/operator/pkg/apis/minio.min.io/v2/helper.go:299 +0x20
github.com/minio/operator/pkg/controller.(*Controller).syncHandler(0x4000540140, {0x4000726708, 0x13})
	github.com/minio/operator/pkg/controller/main-controller.go:742 +0x3d8
github.com/minio/operator/pkg/controller.(*Controller).processNextWorkItem.func1({0x1b8d320?, 0x4000716a80})
	github.com/minio/operator/pkg/controller/main-controller.go:653 +0x1c8
github.com/minio/operator/pkg/controller.(*Controller).processNextWorkItem(0x4000540140)
	github.com/minio/operator/pkg/controller/main-controller.go:665 +0x58
github.com/minio/operator/pkg/controller.(*Controller).runWorker(0x40008c3e88?)
	github.com/minio/operator/pkg/controller/main-controller.go:605 +0x44
k8s.io/apimachinery/pkg/util/wait.BackoffUntil.func1(0x3ea0?)
	k8s.io/apimachinery@v0.26.1/pkg/util/wait/wait.go:157 +0x40
k8s.io/apimachinery/pkg/util/wait.BackoffUntil(0x3acab80?, {0x3ad0ea0, 0x400017f4a0}, 0x1, 0x4000924360)
	k8s.io/apimachinery@v0.26.1/pkg/util/wait/wait.go:158 +0x90
k8s.io/apimachinery/pkg/util/wait.JitterUntil(0x0?, 0x3b9aca00, 0x0, 0x5c?, 0x0?)
	k8s.io/apimachinery@v0.26.1/pkg/util/wait/wait.go:135 +0x80
k8s.io/apimachinery/pkg/util/wait.Until(0x0?, 0x79414?, 0x38321f8?)
	k8s.io/apimachinery@v0.26.1/pkg/util/wait/wait.go:92 +0x28
created by github.com/minio/operator/pkg/controller.leaderRun
	github.com/minio/operator/pkg/controller/main-controller.go:400 +0x2ac

Explanation:

This line below clears the tenant leading to a panic when calling other functions below:

tenant, err = c.checkForPoolDecommission(ctx, key, tenant, tenantConfiguration)

@cniackz cniackz self-assigned this Jun 8, 2023
@cniackz cniackz added the bug Something isn't working label Jun 8, 2023
harshavardhana
harshavardhana requested changes Jun 8, 2023
View reviewed changes
pkg/controller/main-controller.go
@@ -734,7 +734,10 @@ func (c *Controller) syncHandler(key string) error {
}

// Check if we are decommissioning a pool before we ensure defaults, as that would populate a defaulted pool name
tenant, err = c.checkForPoolDecommission(ctx, key, tenant, tenantConfiguration)
temporal_tenant, err := c.checkForPoolDecommission(ctx, key, tenant, tenantConfiguration)
Copy link
Member

Choose a reason for hiding this comment

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

No _ underscore naming allowed in Go.

Copy link
Member

Choose a reason for hiding this comment

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

This still won't solve the problem for the future. The function here incorrectly returns nil, nil

I believe the correct fix is #1625

@jiuker
Copy link
Contributor

jiuker commented Jun 8, 2023 •
edited
Loading

it seems like https://github.com/minio/operator/pull/1625 what's your options?
@harshavardhana @cniackz

harshavardhana
harshavardhana requested changes Jun 8, 2023
View reviewed changes
pkg/controller/main-controller.go
@@ -734,7 +734,10 @@ func (c *Controller) syncHandler(key string) error {
}

// Check if we are decommissioning a pool before we ensure defaults, as that would populate a defaulted pool name
tenant, err = c.checkForPoolDecommission(ctx, key, tenant, tenantConfiguration)
temporal_tenant, err := c.checkForPoolDecommission(ctx, key, tenant, tenantConfiguration)
Copy link
Member

Choose a reason for hiding this comment

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

This still won't solve the problem for the future. The function here incorrectly returns nil, nil

I believe the correct fix is #1625

@pjuarezd
Copy link
Member

Closed in favor of #1625

@pjuarezd pjuarezd closed this Jun 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@harshavardhana harshavardhana harshavardhana requested changes

@pjuarezd pjuarezd Awaiting requested review from pjuarezd

@jiuker jiuker Awaiting requested review from jiuker

@dvaldivia dvaldivia Awaiting requested review from dvaldivia

@allanrogerr allanrogerr Awaiting requested review from allanrogerr

Assignees

@cniackz cniackz

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@cniackz @jiuker @pjuarezd @harshavardhana