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

cmd/openshift-install/create: One shot console access #5336

Merged
merged 1 commit into from
Aug 25, 2022
Merged

cmd/openshift-install/create: One shot console access #5336

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
38 changes: 21 additions & 17 deletions cmd/openshift-install/create.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/sirupsen/logrus"
"github.com/spf13/cobra"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/fields"
"k8s.io/apimachinery/pkg/util/wait"
Expand Down Expand Up @@ -526,8 +527,8 @@ func waitForInitializedCluster(ctx context.Context, config *rest.Config) error {
return errors.Wrap(err, "failed to initialize the cluster")
}

// waitForConsole returns the console URL from the route 'console' in namespace openshift-console
func waitForConsole(ctx context.Context, config *rest.Config) (string, error) {
// getConsole returns the console URL from the route 'console' in namespace openshift-console
func getConsole(ctx context.Context, config *rest.Config) (string, error) {
url := ""
// Need to keep these updated if they change
consoleNamespace := "openshift-console"
Expand All @@ -537,11 +538,8 @@ func waitForConsole(ctx context.Context, config *rest.Config) (string, error) {
return "", errors.Wrap(err, "creating a route client")
}

consoleRouteTimeout := 10 * time.Minute
untilTime := time.Now().Add(consoleRouteTimeout)
logrus.Infof("Waiting up to %v (until %v) for the openshift-console route to be created...",
consoleRouteTimeout, untilTime.Format(time.Kitchen))

consoleRouteTimeout := 2 * time.Minute
logrus.Infof("Checking to see if there is a route at %s/%s...", consoleNamespace, consoleRouteName)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think that there is any value is checking for the console ClusterOperator to determine whether the installer should expect the console route?

Copy link
Member Author

Choose a reason for hiding this comment

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

That would backstop "console operator flubbed their Available handling and claimed Available=True before the route was ready". I'd consider that flub a pretty big console bug though. I guess that means I think it's not the installer's job to backstop the console operator, and that if the console folks are concerned about this potential console failure mode, they could add coverage to their console-specific e2e suite or the OCP-scoped e2e suite to ensure it doesn't happen.

But I'm not too committed to that separation of concerns, so if you'd like a ClusterOperator check as an installer-side backstop, I can add that in.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't have a problem with the approach that you are taking, given that we are now stopping immediately when the console route is not found.

My only gripe is that we are then warning the user that there is no console in the cases where the console is not installed, but that is not relevant yet since we don't yet support not installing the console.

consoleRouteContext, cancel := context.WithTimeout(ctx, consoleRouteTimeout)
defer cancel()
// Poll quickly but only log when the response
Expand All @@ -561,7 +559,11 @@ func waitForConsole(ctx context.Context, config *rest.Config) (string, error) {
} else {
err = err2
}
} else if apierrors.IsNotFound(err) {
logrus.Debug("OpenShift console route does not exist")
cancel()
}

if err != nil {
silenceRemaining--
if silenceRemaining == 0 {
Expand Down Expand Up @@ -594,9 +596,11 @@ func logComplete(directory, consoleURL string) error {
return err
}
logrus.Info("Install complete!")
logrus.Infof("To access the cluster as the system:admin user when using 'oc', run\n export KUBECONFIG=%s", kubeconfig)
logrus.Infof("Access the OpenShift web-console here: %s", consoleURL)
logrus.Infof("Login to the console with user: %q, and password: %q", "kubeadmin", pw)
logrus.Infof("To access the cluster as the system:admin user when using 'oc', run 'export KUBECONFIG=%s'", kubeconfig)
if consoleURL != "" {
logrus.Infof("Access the OpenShift web-console here: %s", consoleURL)
logrus.Infof("Login to the console with user: %q, and password: %q", "kubeadmin", pw)
}
return nil
}

Expand All @@ -605,13 +609,13 @@ func waitForInstallComplete(ctx context.Context, config *rest.Config, directory
return err
}

consoleURL, err := waitForConsole(ctx, config)
if err != nil {
return err
}

if err = addRouterCAToClusterCA(ctx, config, rootOpts.dir); err != nil {
return err
consoleURL, err := getConsole(ctx, config)
if err == nil {
if err = addRouterCAToClusterCA(ctx, config, rootOpts.dir); err != nil {
return err
}
} else {
logrus.Warnf("Cluster does not have a console available: %v", err)
}

return logComplete(rootOpts.dir, consoleURL)
Expand Down