[Liqoctl] Force unpeer command #3126
Conversation
|
Hi @giuliafalaschi. Thanks for your PR! I am @adamjensenbot.
Make sure this PR appears in the liqo changelog, adding one of the following labels:
|
There was a problem hiding this comment.
Hey @giuliafalaschi, thanks for this PR 💯
I added some issues that need to be addressed before merging :)
cmd/liqoctl/cmd/force.go
Outdated
| "github.com/liqotech/liqo/pkg/liqoctl/utils" | ||
| ) | ||
|
|
||
| //TODO: sistemare le descrizioni che dovranno essere inserite nell'help della force |
There was a problem hiding this comment.
| //TODO: sistemare le descrizioni che dovranno essere inserite nell'help della force |
cmd/liqoctl/cmd/force.go
Outdated
| }, | ||
| } | ||
|
|
||
| cmd.PersistentFlags().StringVar(&options.ClusterId, "cluster-id", "", "Force unpeering only on the local cluster") |
There was a problem hiding this comment.
I'd make use the format:
liqoctl force unpeer <CLUSTER_ID>
as we for example fo with the liqoctl unoffload command.
| cmd.PersistentFlags().StringVar(&options.ClusterId, "cluster-id", "", "Force unpeering only on the local cluster") |
cmd/liqoctl/cmd/force.go
Outdated
| Use: "unpeer", | ||
| Short: "Force unpeer a cluster", | ||
| Long: liqoctUnpeerForceLongHelp, | ||
| ValidArgsFunction: completion.ClusterIDs(ctx, f, completion.NoLimit), |
There was a problem hiding this comment.
Allow a single argument and provide suggest for just one argument:
| ValidArgsFunction: completion.ClusterIDs(ctx, f, completion.NoLimit), | |
| Args: cobra.ExactArgs(1), | |
| ValidArgsFunction: completion.ClusterIDs(ctx, f, 1), |
cmd/liqoctl/cmd/force.go
Outdated
| Long: liqoctUnpeerForceLongHelp, | ||
| ValidArgsFunction: completion.ClusterIDs(ctx, f, completion.NoLimit), | ||
|
|
||
| Run: func(_ *cobra.Command, _ []string) { |
There was a problem hiding this comment.
Once we do not use the --cluster-id argument, then we need to pass it via args:
| Run: func(_ *cobra.Command, _ []string) { | |
| Run: func(_ *cobra.Command, args []string) { | |
| options.ClusterId = args[0] |
examples/quick-start/setup.sh
Outdated
|
|
||
| export LIQOCTL="/home/gfalaschi/liqo-giufal/liqo/liqoctl" |
There was a problem hiding this comment.
| export LIQOCTL="/home/gfalaschi/liqo-giufal/liqo/liqoctl" |
pkg/liqoctl/force/unpeer/handler.go
Outdated
| // Options encapsulates the arguments of the info command. | ||
| type Options struct { | ||
| *factory.Factory | ||
| LocalFactory *factory.Factory |
There was a problem hiding this comment.
No need to have LocalFactory, we can just use Factory
| LocalFactory *factory.Factory |
Additionally, as Factory is a promoted field, we can access its fields directly from options, e.g. (o.Printer instead of o.Factory.Printer)
pkg/liqoctl/force/unpeer/handler.go
Outdated
|
|
||
| func NewOptions(localFactory *factory.Factory) *Options { | ||
| return &Options{ | ||
| LocalFactory: localFactory, |
There was a problem hiding this comment.
Because of above this becomes:
| LocalFactory: localFactory, | |
| Factory: localFactory, |
pkg/liqoctl/force/unpeer/handler.go
Outdated
| "liqo.io/foreign-cluster-permanently-unreachable": "true", | ||
| }) | ||
|
|
||
| err = o.LocalFactory.CRClient.Patch(ctx, fc, patch) |
There was a problem hiding this comment.
| err = o.LocalFactory.CRClient.Patch(ctx, fc, patch) | |
| err = o.CRClient.Patch(ctx, fc, patch) |
cmd/liqoctl/cmd/force.go
Outdated
| } | ||
|
|
||
| func newForceCommand(ctx context.Context, f *factory.Factory) *cobra.Command { | ||
| options := unpeer.NewOptions(f) |
There was a problem hiding this comment.
I believe we should initialize options inside newUnpeerForceCommand and not here, so you don't need to pass it as parameter.
giuliafalaschi
force-pushed
the
force-command
branch
from
4b674f3 to
03a4416
Compare
3 participants
Description
I created the ‘force unpeer --cluster-id ’ command, which allows you to force the unpeer command if one of the two clusters is no longer reachable. What this command actually does is set an annotation to the foreigncluster and delete the namespace-tenant of the one invoking the force. These additions will be taken over by the backend, which will perform the actual force unpeer.
Fixes #3051