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

Add warning to deprecate disableCSI through CLI #5918

Merged
merged 1 commit into from
May 26, 2023
Merged

Add warning to deprecate disableCSI through CLI #5918

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 11 additions & 0 deletions pkg/providers/vsphere/vsphere.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,17 @@ func (p *vsphereProvider) PostMoveManagementToBootstrap(_ context.Context, _ *ty
return nil
}

// TODO: Remove this field for v0.17.x, adding a warning as of v0.16.0
func warnIfCSIEnabled(disableCSI bool) {
if !disableCSI {
// Need to add spacing to make the log message look neat
logger.MarkWarning(" Warning: Installing CSI through EKS Anywhere is deprecated. Refer to the official documentation for more details on " +
Copy link
Member

Choose a reason for hiding this comment

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

I think that wording is fine. Saying that something "is deprecated" means that it will be going away in the future, but has not gone away yet.

Copy link
Contributor

Choose a reason for hiding this comment

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

@vivek-koppuru Do we have a ticket users can track? Its useful to include that in warnings. No worries if not.

Copy link
Member Author

Choose a reason for hiding this comment

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

We have this issue #5517 but if we feel that is not the right one, we can create a different one. I am still leaning towards putting it in our documentation and changelog and not here because I don't want to make the message bigger, unless we think that is not a problem.

"the disableCSI field in VSphereDatacenterConfig")
}
}

func (p *vsphereProvider) SetupAndValidateCreateCluster(ctx context.Context, clusterSpec *cluster.Spec) error {
warnIfCSIEnabled(clusterSpec.VSphereDatacenter.Spec.DisableCSI)
if err := p.validator.validateUpgradeRolloutStrategy(clusterSpec); err != nil {
return fmt.Errorf("failed setup and validations: %v", err)
}
Expand Down Expand Up @@ -352,6 +362,7 @@ func (p *vsphereProvider) SetupAndValidateCreateCluster(ctx context.Context, clu
}

func (p *vsphereProvider) SetupAndValidateUpgradeCluster(ctx context.Context, cluster *types.Cluster, clusterSpec *cluster.Spec, _ *cluster.Spec) error {
warnIfCSIEnabled(clusterSpec.VSphereDatacenter.Spec.DisableCSI)
if err := p.validator.validateUpgradeRolloutStrategy(clusterSpec); err != nil {
return fmt.Errorf("failed setup and validations: %v", err)
}
Expand Down
1 change: 1 addition & 0 deletions pkg/providers/vsphere/vsphere_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ func givenEmptyClusterSpec() *cluster.Spec {
s.VersionsBundle.KubeVersion = "1.19"
s.VersionsBundle.EksD.Name = eksd119Release
s.Cluster.Namespace = "test-namespace"
s.VSphereDatacenter = &v1alpha1.VSphereDatacenterConfig{}
})
}

Expand Down