Skip to content

Commit

Permalink
Add timeout param for create machine
Browse files Browse the repository at this point in the history
Add timeout for create machine param, so that we won't have a
fixed time for timeout, user can give param on timeout

Fix Bug #860
  • Loading branch information
jichenjc committed Apr 1, 2019
1 parent 92949fa commit 0977b70
Show file tree
Hide file tree
Showing 7 changed files with 31 additions and 15 deletions.
2 changes: 2 additions & 0 deletions cmd/clusterctl/clusterdeployer/bootstrap/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,15 @@ import "github.com/spf13/pflag"
type Options struct {
Type string
Cleanup bool
TimeoutMachineReady int
ExtraFlags []string
KubeConfig string
}

func (o *Options) AddFlags(fs *pflag.FlagSet) {
fs.StringVarP(&o.Type, "bootstrap-type", "", "none", "The cluster bootstrapper to use.")
fs.BoolVarP(&o.Cleanup, "bootstrap-cluster-cleanup", "", true, "Whether to cleanup the bootstrap cluster after bootstrap.")
fs.IntVarP(&o.TimeoutMachineReady, "bootstrap-machine-ready-timeout", "", 30, "timeout (in minute) for the machine to be ready.")
fs.StringVarP(&o.KubeConfig, "bootstrap-cluster-kubeconfig", "e", "", "Sets the bootstrap cluster to be an existing Kubernetes cluster.")
fs.StringSliceVarP(&o.ExtraFlags, "bootstrap-flags", "", []string{}, "Command line flags to be passed to the chosen bootstrapper")
}
13 changes: 9 additions & 4 deletions cmd/clusterctl/clusterdeployer/clusterclient/clusterclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ type Client interface {
CreateMachineClass(*clusterv1.MachineClass) error
CreateMachineDeployments([]*clusterv1.MachineDeployment, string) error
CreateMachineSets([]*clusterv1.MachineSet, string) error
CreateMachines([]*clusterv1.Machine, string) error
CreateMachines([]*clusterv1.Machine, string, time.Duration) error
Delete(string) error
DeleteClusters(string) error
DeleteNamespace(string) error
Expand Down Expand Up @@ -469,12 +469,17 @@ func (c *client) CreateMachineSets(machineSets []*clusterv1.MachineSet, namespac
return nil
}

func (c *client) CreateMachines(machines []*clusterv1.Machine, namespace string) error {
func (c *client) CreateMachines(machines []*clusterv1.Machine, namespace string, timeout time.Duration) error {
var (
wg sync.WaitGroup
errOnce sync.Once
gerr error
)
// 0 means use default value
if (timeout == 0) {
timeout = timeoutMachineReady
}

// The approach to concurrency here comes from golang.org/x/sync/errgroup.
for _, machine := range machines {
wg.Add(1)
Expand All @@ -490,7 +495,7 @@ func (c *client) CreateMachines(machines []*clusterv1.Machine, namespace string)
return
}

if err := waitForMachineReady(c.clientSet, createdMachine); err != nil {
if err := waitForMachineReady(c.clientSet, createdMachine, timeoutMachineReady); err != nil {
errOnce.Do(func() { gerr = err })
}
}(machine)
Expand Down Expand Up @@ -967,7 +972,7 @@ func waitForClusterResourceReady(cs clientset.Interface) error {
})
}

func waitForMachineReady(cs clientset.Interface, machine *clusterv1.Machine) error {
func waitForMachineReady(cs clientset.Interface, machine *clusterv1.Machine, timeoutMachineReady time.Duration) error {
err := util.PollImmediate(retryIntervalResourceReady, timeoutMachineReady, func() (bool, error) {
klog.V(2).Infof("Waiting for Machine %v to become ready...", machine.Name)
m, err := cs.ClusterV1alpha1().Machines(machine.Namespace).Get(machine.Name, metav1.GetOptions{})
Expand Down
10 changes: 7 additions & 3 deletions cmd/clusterctl/clusterdeployer/clusterdeployer.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package clusterdeployer
import (
"fmt"
"strings"
"time"

"github.com/pkg/errors"
"k8s.io/klog"
Expand All @@ -36,6 +37,7 @@ type ClusterDeployer struct {
addonComponents string
bootstrapComponents string
cleanupBootstrapCluster bool
timeoutMachineReady time.Duration
}

func New(
Expand All @@ -44,14 +46,16 @@ func New(
providerComponents string,
addonComponents string,
bootstrapComponents string,
cleanupBootstrapCluster bool) *ClusterDeployer {
cleanupBootstrapCluster bool,
timeoutMachineReady time.Duration) *ClusterDeployer {
return &ClusterDeployer{
bootstrapProvisioner: bootstrapProvisioner,
clientFactory: clientFactory,
providerComponents: providerComponents,
addonComponents: addonComponents,
bootstrapComponents: bootstrapComponents,
cleanupBootstrapCluster: cleanupBootstrapCluster,
timeoutMachineReady: timeoutMachineReady,
}
}

Expand Down Expand Up @@ -90,7 +94,7 @@ func (d *ClusterDeployer) Create(cluster *clusterv1.Cluster, machines []*cluster
}

klog.Infof("Creating control plane %v in namespace %q", controlPlaneMachine.Name, cluster.Namespace)
if err := phases.ApplyMachines(bootstrapClient, cluster.Namespace, []*clusterv1.Machine{controlPlaneMachine}); err != nil {
if err := phases.ApplyMachines(bootstrapClient, cluster.Namespace, []*clusterv1.Machine{controlPlaneMachine}, d.timeoutMachineReady); err != nil {
return errors.Wrap(err, "unable to create control plane machine")
}

Expand Down Expand Up @@ -136,7 +140,7 @@ func (d *ClusterDeployer) Create(cluster *clusterv1.Cluster, machines []*cluster
}

klog.Info("Creating node machines in target cluster.")
if err := phases.ApplyMachines(targetClient, cluster.Namespace, nodes); err != nil {
if err := phases.ApplyMachines(targetClient, cluster.Namespace, nodes, d.timeoutMachineReady); err != nil {
return errors.Wrap(err, "unable to create node machines")
}

Expand Down
6 changes: 3 additions & 3 deletions cmd/clusterctl/clusterdeployer/clusterdeployer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -857,7 +857,7 @@ func TestClusterCreate(t *testing.T) {

pcStore := mockProviderComponentsStore{}
pcFactory := mockProviderComponentsStoreFactory{NewFromCoreclientsetPCStore: &pcStore}
d := New(p, f, "", "", bootstrapComponent, testcase.cleanupExternal)
d := New(p, f, "", "", bootstrapComponent, testcase.cleanupExternal, 0)

inputMachines := make(map[string][]*clusterv1.Machine)

Expand Down Expand Up @@ -972,7 +972,7 @@ func TestCreateProviderComponentsScenarios(t *testing.T) {
pcFactory := mockProviderComponentsStoreFactory{NewFromCoreclientsetPCStore: &tc.pcStore}
providerComponentsYaml := "---\nyaml: definition"
addonsYaml := "---\nyaml: definition"
d := New(p, f, providerComponentsYaml, addonsYaml, "", false)
d := New(p, f, providerComponentsYaml, addonsYaml, "", false, 0)
err := d.Create(inputCluster, inputMachines, pd, kubeconfigOut, &pcFactory)
if err == nil && tc.expectedError != "" {
t.Fatalf("error mismatch: got '%v', want '%v'", err, tc.expectedError)
Expand Down Expand Up @@ -1106,7 +1106,7 @@ func TestDeleteCleanupExternalCluster(t *testing.T) {
f := newTestClusterClientFactory()
f.clusterClients[bootstrapKubeconfig] = tc.bootstrapClient
f.clusterClients[targetKubeconfig] = tc.targetClient
d := New(p, f, "", "", "", tc.cleanupExternalCluster)
d := New(p, f, "", "", "", tc.cleanupExternalCluster, 0)
err := d.Delete(tc.targetClient)
if err != nil || tc.expectedErrorMessage != "" {
if err == nil {
Expand Down
4 changes: 3 additions & 1 deletion cmd/clusterctl/cmd/create_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package cmd

import (
"io/ioutil"
"time"

"github.com/pkg/errors"
"github.com/spf13/cobra"
Expand Down Expand Up @@ -107,7 +108,8 @@ func RunCreate(co *CreateOptions) error {
string(pc),
string(ac),
string(bc),
co.BootstrapFlags.Cleanup)
co.BootstrapFlags.Cleanup,
time.Duration(co.BootstrapFlags.TimeoutMachineReady) * time.Minute)

return d.Create(c, m, pd, co.KubeconfigOutput, pcsFactory)
}
Expand Down
6 changes: 4 additions & 2 deletions cmd/clusterctl/phases/applymachines.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,15 @@ limitations under the License.
package phases

import (
"time"

"github.com/pkg/errors"
"k8s.io/klog"
"sigs.k8s.io/cluster-api/cmd/clusterctl/clusterdeployer/clusterclient"
clusterv1 "sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1"
)

func ApplyMachines(client clusterclient.Client, namespace string, machines []*clusterv1.Machine) error {
func ApplyMachines(client clusterclient.Client, namespace string, machines []*clusterv1.Machine, timeoutCreateMachines time.Duration) error {
if namespace == "" {
namespace = client.GetContextNamespace()
}
Expand All @@ -34,7 +36,7 @@ func ApplyMachines(client clusterclient.Client, namespace string, machines []*cl
}

klog.Infof("Creating machines in namespace %q", namespace)
if err := client.CreateMachines(machines, namespace); err != nil {
if err := client.CreateMachines(machines, namespace, timeoutCreateMachines); err != nil {
return err
}

Expand Down
5 changes: 3 additions & 2 deletions cmd/clusterctl/phases/pivot.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package phases
import (
"io"
"strings"
"time"

"github.com/pkg/errors"
appsv1 "k8s.io/api/apps/v1"
Expand Down Expand Up @@ -53,7 +54,7 @@ type targetClient interface {
CreateClusterObject(*clusterv1.Cluster) error
CreateMachineClass(*clusterv1.MachineClass) error
CreateMachineDeployments([]*clusterv1.MachineDeployment, string) error
CreateMachines([]*clusterv1.Machine, string) error
CreateMachines([]*clusterv1.Machine, string, time.Duration) error
CreateMachineSets([]*clusterv1.MachineSet, string) error
EnsureNamespace(string) error
GetMachineDeployment(namespace, name string) (*clusterv1.MachineDeployment, error)
Expand Down Expand Up @@ -390,7 +391,7 @@ func moveMachine(from sourceClient, to targetClient, m *clusterv1.Machine) error
// Remove owner reference. This currently assumes that the only owner references would be a MachineSet and/or a Cluster.
m.SetOwnerReferences(nil)

if err := to.CreateMachines([]*clusterv1.Machine{m}, m.Namespace); err != nil {
if err := to.CreateMachines([]*clusterv1.Machine{m}, m.Namespace, 0); err != nil {
return errors.Wrapf(err, "error copying Machine %s/%s to target cluster", m.Namespace, m.Name)
}
if err := from.ForceDeleteMachine(m.Namespace, m.Name); err != nil {
Expand Down

0 comments on commit 0977b70

Please sign in to comment.