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

Removing credsSecret required field in tenant spec #1009

Merged
merged 1 commit into from
Aug 31, 2022

Conversation

Alevsk
Copy link
Contributor

@Alevsk Alevsk commented Feb 5, 2022

  • Removing required credsSecret field in tenant spec
  • Migration from creddsSecret to Configuration field for old tenants

Signed-off-by: Lenin Alevski alevsk.8772@gmail.com

@Alevsk Alevsk self-assigned this Feb 5, 2022
@Alevsk Alevsk force-pushed the remove-credssecret branch 3 times, most recently from 63af002 to 3e8f1fb Compare February 5, 2022 23:32
pkg/controller/cluster/main-controller.go Outdated Show resolved Hide resolved
pkg/controller/cluster/upgrades.go Outdated Show resolved Hide resolved
examples/kustomization/tenant-test/tenantNamePatch.yaml Outdated Show resolved Hide resolved
examples/kustomization/tenant-test/tenant-credssecret.yaml Outdated Show resolved Hide resolved
examples/kustomization/tenant-test/kustomization.yaml Outdated Show resolved Hide resolved
@Alevsk Alevsk force-pushed the remove-credssecret branch 2 times, most recently from 836e5c5 to 0926ccd Compare February 8, 2022 00:51
harshavardhana
harshavardhana previously approved these changes Feb 8, 2022
@Alevsk Alevsk force-pushed the remove-credssecret branch from 0c39bc2 to 1c9740a Compare April 5, 2022 17:13
@Alevsk Alevsk requested a review from harshavardhana April 5, 2022 17:13
harshavardhana
harshavardhana previously approved these changes Apr 5, 2022
@harshavardhana
Copy link
Member

PTAL @Alevsk at conflicts

@harshavardhana
Copy link
Member

Can we rebase this and take it in @Alevsk ?

@Alevsk
Copy link
Contributor Author

Alevsk commented May 31, 2022

Can we rebase this and take it in @Alevsk ?

Working on this today @harshavardhana

@Alevsk Alevsk force-pushed the remove-credssecret branch from c2a4b0d to bb0720c Compare June 28, 2022 02:44
@Alevsk Alevsk requested a review from harshavardhana June 28, 2022 02:45
Copy link
Member

@harshavardhana harshavardhana left a comment

Choose a reason for hiding this comment

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

LGTM overall, few comments

pkg/controller/cluster/upgrades.go Outdated Show resolved Hide resolved
pkg/controller/cluster/upgrades.go Outdated Show resolved Hide resolved
pkg/controller/cluster/upgrades.go Outdated Show resolved Hide resolved
pkg/controller/cluster/upgrades.go Outdated Show resolved Hide resolved
pkg/controller/cluster/main-controller.go Outdated Show resolved Hide resolved
pkg/controller/cluster/main-controller.go Outdated Show resolved Hide resolved
pkg/controller/cluster/main-controller.go Outdated Show resolved Hide resolved
pkg/controller/cluster/main-controller.go Outdated Show resolved Hide resolved
pkg/apis/minio.min.io/v2/helper.go Outdated Show resolved Hide resolved
pkg/apis/minio.min.io/v1/helper.go Show resolved Hide resolved
@Alevsk Alevsk force-pushed the remove-credssecret branch from bb0720c to 940b048 Compare June 28, 2022 22:26
@Alevsk Alevsk requested a review from harshavardhana June 28, 2022 22:27
harshavardhana
harshavardhana previously approved these changes Jun 28, 2022
@Alevsk Alevsk force-pushed the remove-credssecret branch from 940b048 to 85be30b Compare July 20, 2022 18:58
harshavardhana
harshavardhana previously approved these changes Jul 20, 2022
@cniackz cniackz self-requested a review August 9, 2022 17:46
Copy link
Contributor

@cniackz cniackz left a comment

Choose a reason for hiding this comment

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

@Alevsk please resolve the conflicts 👍

Copy link
Collaborator

@dvaldivia dvaldivia left a comment

Choose a reason for hiding this comment

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

I'm hitting an NPE with a tenant which has no config file

goroutine 361 [running]:
k8s.io/apimachinery/pkg/util/runtime.HandleCrash({0x0, 0x0, 0x4dd574})
    k8s.io/apimachinery@v0.20.6/pkg/util/runtime/runtime.go:55 +0xd8
panic({0x16ad080, 0x2759220})
    runtime/panic.go:1038 +0x215
k8s.io/apimachinery/pkg/util/runtime.HandleCrash({0x0, 0x0, 0xc000294250})
    k8s.io/apimachinery@v0.20.6/pkg/util/runtime/runtime.go:55 +0xd8
panic({0x16ad080, 0x2759220})
    runtime/panic.go:1038 +0x215
github.com/minio/operator/pkg/controller/cluster.(*Controller).checkForUpgrades(0xc000450480, {0x1af9218, 0xc000042028}, 0xc000e5f180)
    github.com/minio/operator/pkg/controller/cluster/upgrades.go:93 +0xaca
github.com/minio/operator/pkg/controller/cluster.(*Controller).syncHandler(0xc000450480, {0xc001235bc0, 0x17})
    github.com/minio/operator/pkg/controller/cluster/main-controller.go:650 +0x5ab
github.com/minio/operator/pkg/controller/cluster.(*Controller).processNextWorkItem.func1({0x163e060, 0xc000294250})
    github.com/minio/operator/pkg/controller/cluster/main-controller.go:560 +0x245
github.com/minio/operator/pkg/controller/cluster.(*Controller).processNextWorkItem(0xc000450480)
    github.com/minio/operator/pkg/controller/cluster/main-controller.go:572 +0x62
github.com/minio/operator/pkg/controller/cluster.(*Controller).runWorker(0x0)
    github.com/minio/operator/pkg/controller/cluster/main-controller.go:512 +0x70
k8s.io/apimachinery/pkg/util/wait.BackoffUntil.func1(0x7fe77453cd60)
    k8s.io/apimachinery@v0.20.6/pkg/util/wait/wait.go:155 +0x67
k8s.io/apimachinery/pkg/util/wait.BackoffUntil(0xc0003e90e0, {0x1acbfc0, 0xc0006f4e70}, 0x1, 0xc00008f620)
    k8s.io/apimachinery@v0.20.6/pkg/util/wait/wait.go:156 +0xb6
k8s.io/apimachinery/pkg/util/wait.JitterUntil(0xc000998520, 0x3b9aca00, 0x0, 0x28, 0xc0003e9140)
    k8s.io/apimachinery@v0.20.6/pkg/util/wait/wait.go:133 +0x89
k8s.io/apimachinery/pkg/util/wait.Until(0xc000998590, 0xc0001c6480, 0xc00099c7e0)
    k8s.io/apimachinery@v0.20.6/pkg/util/wait/wait.go:90 +0x25
created by github.com/minio/operator/pkg/controller/cluster.(*Controller).Start.func1
    github.com/minio/operator/pkg/controller/cluster/main-controller.go:397 +0x3c5
Stream closed EOF for minio-operator/minio-operator-6f4f49887d-55pj4 (minio-operator)

@Alevsk
Copy link
Contributor Author

Alevsk commented Aug 25, 2022

I'm hitting an NPE with a tenant which has no config file

goroutine 361 [running]:
k8s.io/apimachinery/pkg/util/runtime.HandleCrash({0x0, 0x0, 0x4dd574})
    k8s.io/apimachinery@v0.20.6/pkg/util/runtime/runtime.go:55 +0xd8
panic({0x16ad080, 0x2759220})
    runtime/panic.go:1038 +0x215
k8s.io/apimachinery/pkg/util/runtime.HandleCrash({0x0, 0x0, 0xc000294250})
    k8s.io/apimachinery@v0.20.6/pkg/util/runtime/runtime.go:55 +0xd8
panic({0x16ad080, 0x2759220})
    runtime/panic.go:1038 +0x215
github.com/minio/operator/pkg/controller/cluster.(*Controller).checkForUpgrades(0xc000450480, {0x1af9218, 0xc000042028}, 0xc000e5f180)
    github.com/minio/operator/pkg/controller/cluster/upgrades.go:93 +0xaca
github.com/minio/operator/pkg/controller/cluster.(*Controller).syncHandler(0xc000450480, {0xc001235bc0, 0x17})
    github.com/minio/operator/pkg/controller/cluster/main-controller.go:650 +0x5ab
github.com/minio/operator/pkg/controller/cluster.(*Controller).processNextWorkItem.func1({0x163e060, 0xc000294250})
    github.com/minio/operator/pkg/controller/cluster/main-controller.go:560 +0x245
github.com/minio/operator/pkg/controller/cluster.(*Controller).processNextWorkItem(0xc000450480)
    github.com/minio/operator/pkg/controller/cluster/main-controller.go:572 +0x62
github.com/minio/operator/pkg/controller/cluster.(*Controller).runWorker(0x0)
    github.com/minio/operator/pkg/controller/cluster/main-controller.go:512 +0x70
k8s.io/apimachinery/pkg/util/wait.BackoffUntil.func1(0x7fe77453cd60)
    k8s.io/apimachinery@v0.20.6/pkg/util/wait/wait.go:155 +0x67
k8s.io/apimachinery/pkg/util/wait.BackoffUntil(0xc0003e90e0, {0x1acbfc0, 0xc0006f4e70}, 0x1, 0xc00008f620)
    k8s.io/apimachinery@v0.20.6/pkg/util/wait/wait.go:156 +0xb6
k8s.io/apimachinery/pkg/util/wait.JitterUntil(0xc000998520, 0x3b9aca00, 0x0, 0x28, 0xc0003e9140)
    k8s.io/apimachinery@v0.20.6/pkg/util/wait/wait.go:133 +0x89
k8s.io/apimachinery/pkg/util/wait.Until(0xc000998590, 0xc0001c6480, 0xc00099c7e0)
    k8s.io/apimachinery@v0.20.6/pkg/util/wait/wait.go:90 +0x25
created by github.com/minio/operator/pkg/controller/cluster.(*Controller).Start.func1
    github.com/minio/operator/pkg/controller/cluster/main-controller.go:397 +0x3c5
Stream closed EOF for minio-operator/minio-operator-6f4f49887d-55pj4 (minio-operator)

If tenant has no config file we should throw an error and stop execution for that tenant, meaning if theres a version of that tenant already running thats fine, but we should not apply any new incoming changes, that sounds good?

@Alevsk Alevsk force-pushed the remove-credssecret branch 3 times, most recently from 13e954e to eb372fc Compare August 26, 2022 20:05
@Alevsk Alevsk force-pushed the remove-credssecret branch from eb372fc to 4bd3b42 Compare August 29, 2022 17:58
@Alevsk Alevsk force-pushed the remove-credssecret branch 2 times, most recently from 49a5a93 to aa5879a Compare August 30, 2022 16:33
- Removing required credsSecret field in tenant spec
- Migration from creddsSecret to Configuration field for old tenants
- Added helm lint verification to install_tenant test

Signed-off-by: Lenin Alevski <alevsk.8772@gmail.com>
@Alevsk Alevsk force-pushed the remove-credssecret branch from aa5879a to bd32c2f Compare August 30, 2022 20:14
@harshavardhana
Copy link
Member

Are we ready to make v5.0.0 @dvaldivia ?

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

Successfully merging this pull request may close these issues.

4 participants