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

pkg/start/start: Drop bootstrapPodsRunningTimeout #24

Closed
wants to merge 1 commit into from
Closed

pkg/start/start: Drop bootstrapPodsRunningTimeout #24

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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion cmd/cluster-bootstrap/start.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package main

import (
"context"
"errors"
"strings"

Expand Down Expand Up @@ -47,6 +48,8 @@ func init() {
}

func runCmdStart(cmd *cobra.Command, args []string) error {
ctx := context.Background()

podPrefixes, err := parsePodPrefixes(startOpts.requiredPodClauses)
if err != nil {
return err
Expand All @@ -64,7 +67,7 @@ func runCmdStart(cmd *cobra.Command, args []string) error {
return err
}

return bk.Run()
return bk.Run(ctx)
}

// parsePodPrefixes parses <ns>/<pod-prefix> or <desc>:<ns>/<pod-prefix>|... into a map with
Expand Down
17 changes: 6 additions & 11 deletions pkg/start/start.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ import (
)

const (
// how long we wait until the bootstrap pods to be running
bootstrapPodsRunningTimeout = 20 * time.Minute
// how long we wait until the assets must all be created
assetsCreatedTimeout = 60 * time.Minute
)
Expand Down Expand Up @@ -57,7 +55,7 @@ func NewStartCommand(config Config) (*startCommand, error) {
}, nil
}

func (b *startCommand) Run() error {
func (b *startCommand) Run(ctx context.Context) error {
restConfig, err := clientcmd.BuildConfigFromFlags("", filepath.Join(b.assetDir, assetPathAdminKubeConfig))
if err != nil {
return err
Expand Down Expand Up @@ -121,13 +119,12 @@ func (b *startCommand) Run() error {
}()
return &done
}
ctx, cancel := context.WithTimeout(context.TODO(), bootstrapPodsRunningTimeout)
assetContext, cancel := context.WithTimeout(ctx, assetsCreatedTimeout)
defer cancel()
assetsDone := createAssetsInBackground(ctx, cancel, localClientConfig)
assetsDone := createAssetsInBackground(assetContext, cancel, localClientConfig)
if err = waitUntilPodsRunning(ctx, client, b.requiredPodPrefixes); err != nil {
return err
}
cancel()
Copy link
Contributor

Choose a reason for hiding this comment

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

this doesn't work. We break the switch over logic this way. We have to force createAssetsInBackground to stop creating assets. We did this with this cancel() call, and restarted asset creation afterwards, potentially with the ELB such that we can shut down the bootstrap control plane.

Copy link
Contributor

Choose a reason for hiding this comment

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

To fix this: wrap assetContext into another context, cancel it here and then use the inner context below.

assetsDone.Wait()

// notify installer that we are ready to tear down the temporary bootstrap control plane
Expand All @@ -137,14 +134,12 @@ func (b *startCommand) Run() error {
}

// continue with assets
ctx, cancel = context.WithTimeout(context.Background(), assetsCreatedTimeout)
defer cancel()
if b.earlyTearDown {
// switch over to ELB client and continue with the assets
assetsDone = createAssetsInBackground(ctx, cancel, restConfig)
assetsDone = createAssetsInBackground(assetContext, cancel, restConfig)
} else {
// we don't tear down the local control plane early. So we can keep using it and enjoy the speed up.
assetsDone = createAssetsInBackground(ctx, cancel, localClientConfig)
assetsDone = createAssetsInBackground(assetContext, cancel, localClientConfig)
}

// optionally wait for tear down event coming from the installer. This is necessary to
Expand All @@ -155,7 +150,7 @@ func (b *startCommand) Run() error {
return fmt.Errorf("tear down event name of format <namespace>/<event-name> expected, got: %q", b.waitForTearDownEvent)
}
ns, name := ss[0], ss[1]
if err := waitForEvent(context.TODO(), client, ns, name); err != nil {
if err := waitForEvent(ctx, client, ns, name); err != nil {
return err
}
UserOutput("Got %s event.", b.waitForTearDownEvent)
Expand Down