Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions Gopkg.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions Gopkg.toml
Original file line number Diff line number Diff line change
Expand Up @@ -68,3 +68,7 @@ version="v1.4.7"
[[constraint]]
name = "k8s.io/apiserver"
version="kubernetes-1.11.2"

[[constraint]]
branch = "master"
name = "github.com/openshift/api"
5 changes: 5 additions & 0 deletions cmd/manager/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/runtime/signals"

_ "github.com/openshift/generic-admission-server/pkg/cmd"
openshiftapiv1 "github.com/openshift/api/config/v1"
)

const (
Expand Down Expand Up @@ -78,6 +79,10 @@ func NewRootCommand() *cobra.Command {
log.Fatal(err)
}

if err := openshiftapiv1.Install(mgr.GetScheme()); err != nil {
log.Fatal(err)
}

// Setup all Controllers
if err := controller.AddToManager(mgr); err != nil {
log.Fatal(err)
Expand Down
57 changes: 57 additions & 0 deletions config/crds/hive_v1alpha1_clusterdeployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -265,13 +265,70 @@ spec:
type: object
apiURL:
type: string
clusterVersionStatus:
properties:
availableUpdates:
items:
properties:
payload:
type: string
version:
type: string
required:
- version
- payload
type: object
type: array
conditions:
items:
properties:
lastTransitionTime:
format: date-time
type: string
message:
type: string
reason:
type: string
status:
type: string
type:
type: string
required:
- type
- status
- lastTransitionTime
type: object
type: array
current:
properties:
payload:
type: string
version:
type: string
required:
- version
- payload
type: object
generation:
format: int64
type: integer
versionHash:
type: string
required:
- current
- generation
- versionHash
- conditions
- availableUpdates
type: object
installed:
type: boolean
webConsoleURL:
type: string
required:
- installed
- adminKubeconfigSecret
- clusterVersionStatus
- apiURL
- webConsoleURL
type: object
Expand Down
4 changes: 4 additions & 0 deletions pkg/apis/hive/v1alpha1/clusterdeployment_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package v1alpha1

import (
openshiftapiv1 "github.com/openshift/api/config/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)
Expand Down Expand Up @@ -86,6 +87,9 @@ type ClusterDeploymentStatus struct {
// AdminKubeconfigSecret references the secret containing the admin kubeconfig for this cluster.
AdminKubeconfigSecret corev1.LocalObjectReference `json:"adminKubeconfigSecret"`

// ClusterVersionStatus will hold a copy of the remote cluster's ClusterVersion.Status
ClusterVersionStatus openshiftapiv1.ClusterVersionStatus `json:"clusterVersionStatus"`

Copy link
Contributor

Choose a reason for hiding this comment

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

Realistically I think treating this as just a string will probably get us into a spot where we need to break API compatibility in the near future. We should make this a struct, the Update struct also has a Payload which looks really interesting for our purposes as well and we should.

Should we pull the FULL ClusterVersionStatus? Or just make our own and copy over parts of it? Looking through https://github.com/openshift/api/blob/master/config/v1/types_cluster_version.go#L75 we're not really interested in the Generation or VersionHash but the Conditions and the AvailableUpdates could be extremely useful. I'd be tempted to re-use their type and copy the whole thing.

Thoughts?

CC @csrwng in case he has ideas as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree, I think there's usefulness in embedding the entire status struct

// APIURL is the URL where the cluster's API can be accessed.
APIURL string `json:"apiURL"`

Expand Down
59 changes: 58 additions & 1 deletion pkg/controller/clusterdeployment/clusterdeployment_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@ import (
"sigs.k8s.io/controller-runtime/pkg/reconcile"
"sigs.k8s.io/controller-runtime/pkg/source"

openshiftapiv1 "github.com/openshift/api/config/v1"
hivev1 "github.com/openshift/hive/pkg/apis/hive/v1alpha1"
controllerutils "github.com/openshift/hive/pkg/controller/utils"
"github.com/openshift/hive/pkg/install"
)

Expand All @@ -63,6 +65,9 @@ const (
adminCredsSecretPasswordKey = "password"
pullSecretKey = ".dockercfg"
adminSSHKeySecretKey = "ssh-publickey"
adminKubeconfigKey = "kubeconfig"
clusterVersionObjectName = "version"
clusterVersionUnknown = "undef"
)

// Add creates a new ClusterDeployment Controller and adds it to the Manager with default RBAC. The Manager will set fields on the Controller
Expand All @@ -73,7 +78,12 @@ func Add(mgr manager.Manager) error {

// NewReconciler returns a new reconcile.Reconciler
func NewReconciler(mgr manager.Manager) reconcile.Reconciler {
return &ReconcileClusterDeployment{Client: mgr.GetClient(), scheme: mgr.GetScheme(), amiLookupFunc: lookupAMI}
return &ReconcileClusterDeployment{
Client: mgr.GetClient(),
scheme: mgr.GetScheme(),
amiLookupFunc: lookupAMI,
remoteClusterAPIClientBuilder: controllerutils.BuildClusterAPIClientFromKubeconfig,
}
}

// AddToManager adds a new Controller to mgr with r as the reconcile.Reconciler
Expand Down Expand Up @@ -109,6 +119,10 @@ type ReconcileClusterDeployment struct {
client.Client
scheme *runtime.Scheme
amiLookupFunc func(cd *hivev1.ClusterDeployment) (string, error)

// remoteClusterAPIClientBuilder is a function pointer to the function that builds a client for the
// remote cluster's cluster-api
remoteClusterAPIClientBuilder func(string) (client.Client, error)
}

// Reconcile reads that state of the cluster for a ClusterDeployment object and makes changes based on the state read
Expand Down Expand Up @@ -352,6 +366,34 @@ func (r *ReconcileClusterDeployment) updateClusterDeploymentStatus(cd *hivev1.Cl
cd.Status.WebConsoleURL = u.String()
}

// Update remote cluster's version into our status
if cd.Status.AdminKubeconfigSecret.Name != "" {
kubeconfig, err := r.loadSecretData(cd.Status.AdminKubeconfigSecret.Name, cd.Namespace, adminKubeconfigKey)
if err != nil {
cdLog.WithError(err).Error("error loading remote cluster's kubeconfig")
return err
}

remoteClusterAPIClient, err := r.remoteClusterAPIClientBuilder(kubeconfig)
if err != nil {
cdLog.WithError(err).Error("error building remote cluster-api client connection")
return err
}

remoteClusterVersion := &openshiftapiv1.ClusterVersion{}
err = remoteClusterAPIClient.Get(context.Background(),
types.NamespacedName{Name: clusterVersionObjectName},
remoteClusterVersion)
if err != nil {
cdLog.WithError(err).Error("error fetching remote clusterversion object")
return err
}

cdLog.Debugf("remote cluster version status: %+v", remoteClusterVersion.Status)
fixupEmptyClusterVersionFields(&remoteClusterVersion.Status)
remoteClusterVersion.Status.DeepCopyInto(&cd.Status.ClusterVersionStatus)
}

// Update cluster deployment status if changed:
if !reflect.DeepEqual(cd.Status, origCD.Status) {
cdLog.Infof("status has changed, updating cluster deployment")
Expand Down Expand Up @@ -604,3 +646,18 @@ func DeleteFinalizer(object metav1.Object, finalizer string) {
finalizers.Delete(finalizer)
object.SetFinalizers(finalizers.List())
}

func fixupEmptyClusterVersionFields(clusterVersionStatus *openshiftapiv1.ClusterVersionStatus) {

// Fetching clusterVersion object can results in nil clusterVersion.Status.AvailableUpdates
// and clusterVersion.Status.Conditions.
// Place an empty list if needed to satisfy the object validation.

if clusterVersionStatus.AvailableUpdates == nil {
clusterVersionStatus.AvailableUpdates = []openshiftapiv1.Update{}
}

if clusterVersionStatus.Conditions == nil {
clusterVersionStatus.Conditions = []openshiftapiv1.ClusterOperatorStatusCondition{}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"sigs.k8s.io/controller-runtime/pkg/reconcile"

openshiftapiv1 "github.com/openshift/api/config/v1"
"github.com/openshift/hive/pkg/apis"
hivev1 "github.com/openshift/hive/pkg/apis/hive/v1alpha1"
"github.com/openshift/hive/pkg/install"
Expand All @@ -61,6 +62,8 @@ const (
server: https://bar-api.clusters.example.com:6443
name: bar
`
testRemoteClusterCurrentVersion = "4.0.0"
remoteClusterVersionObjectName = "version"
)

func init() {
Expand All @@ -69,6 +72,7 @@ func init() {

func TestClusterDeploymentReconcile(t *testing.T) {
apis.AddToScheme(scheme.Scheme)
openshiftapiv1.Install(scheme.Scheme)

// Utility function to get the test CD from the fake client
getCD := func(c client.Client) *hivev1.ClusterDeployment {
Expand All @@ -79,6 +83,7 @@ func TestClusterDeploymentReconcile(t *testing.T) {
}
return nil
}

getJob := func(c client.Client, name string) *batchv1.Job {
job := &batchv1.Job{}
err := c.Get(context.TODO(), client.ObjectKey{Name: name, Namespace: testNamespace}, job)
Expand Down Expand Up @@ -182,8 +187,27 @@ func TestClusterDeploymentReconcile(t *testing.T) {
},
validate: func(c client.Client, t *testing.T) {
cd := getCD(c)
assert.Equal(t, cd.Status.APIURL, "https://bar-api.clusters.example.com:6443")
assert.Equal(t, cd.Status.WebConsoleURL, "https://bar-api.clusters.example.com:6443/console")
assert.Equal(t, "https://bar-api.clusters.example.com:6443", cd.Status.APIURL)
assert.Equal(t, "https://bar-api.clusters.example.com:6443/console", cd.Status.WebConsoleURL)
Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch.

},
},
{
name: "Fetch remote cluster version status into clusterdeployment status",
existing: []runtime.Object{
func() *hivev1.ClusterDeployment {
cd := testClusterDeployment()
cd.Status.AdminKubeconfigSecret = corev1.LocalObjectReference{Name: adminKubeconfigSecret}
return cd
}(),
testInstallJob(),
testSecret(adminKubeconfigSecret, "kubeconfig", adminKubeconfig),
testSecret(adminPasswordSecret, adminCredsSecretPasswordKey, "password"),
testSecret(pullSecretSecret, pullSecretKey, "{}"),
testSecret(sshKeySecret, adminSSHKeySecretKey, "fakesshkey"),
},
validate: func(c client.Client, t *testing.T) {
cd := getCD(c)
assert.Equal(t, "4.0.0", cd.Status.ClusterVersionStatus.Current.Version)
},
},
{
Expand Down Expand Up @@ -296,6 +320,7 @@ func TestClusterDeploymentReconcile(t *testing.T) {
amiLookupFunc: func(cd *hivev1.ClusterDeployment) (string, error) {
return testAMI, nil
},
remoteClusterAPIClientBuilder: testRemoteClusterAPIClientBuilder,
}

_, err := rcd.Reconcile(reconcile.Request{
Expand Down Expand Up @@ -446,3 +471,28 @@ func testSecret(name, key, value string) *corev1.Secret {
}
return s
}

func testRemoteClusterAPIClientBuilder(secretData string) (client.Client, error) {
remoteClusterVersion := &openshiftapiv1.ClusterVersion{
ObjectMeta: metav1.ObjectMeta{
Name: remoteClusterVersionObjectName,
},
}
remoteClusterVersion.Status = testRemoteClusterVersionStatus()

remoteClient := fake.NewFakeClient(remoteClusterVersion)

return remoteClient, nil
}

func testRemoteClusterVersionStatus() openshiftapiv1.ClusterVersionStatus {
status := openshiftapiv1.ClusterVersionStatus{
Current: openshiftapiv1.Update{
Version: testRemoteClusterCurrentVersion,
Payload: "TESTPAYLOAD",
},
Generation: 123456789,
VersionHash: "TESTVERSIONHASH",
}
return status
}
24 changes: 2 additions & 22 deletions pkg/controller/remotemachineset/remotemachineset_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/tools/clientcmd"

"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller"
Expand All @@ -48,6 +47,7 @@ import (

hivev1 "github.com/openshift/hive/pkg/apis/hive/v1alpha1"
"github.com/openshift/hive/pkg/awsclient"
controllerutils "github.com/openshift/hive/pkg/controller/utils"
"github.com/openshift/hive/pkg/install"
)

Expand Down Expand Up @@ -76,7 +76,7 @@ func NewReconciler(mgr manager.Manager) reconcile.Reconciler {
Client: mgr.GetClient(),
scheme: mgr.GetScheme(),
logger: log.WithField("controller", controllerName),
remoteClusterAPIClientBuilder: buildRemoteClusterAPIClient,
remoteClusterAPIClientBuilder: controllerutils.BuildClusterAPIClientFromKubeconfig,
awsClientBuilder: awsclient.NewClient,
}
}
Expand Down Expand Up @@ -329,26 +329,6 @@ func workerPool(pools []installtypes.MachinePool) installtypes.MachinePool {
return installtypes.MachinePool{}
}

func buildRemoteClusterAPIClient(secretData string) (client.Client, error) {
config, err := clientcmd.Load([]byte(secretData))
if err != nil {
return nil, err
}
kubeConfig := clientcmd.NewDefaultClientConfig(*config, &clientcmd.ConfigOverrides{})
cfg, err := kubeConfig.ClientConfig()
if err != nil {
return nil, err
}

scheme, err := capiv1.SchemeBuilder.Build()
if err != nil {
return nil, err
}
return client.New(cfg, client.Options{
Scheme: scheme,
})
}

func (r *ReconcileRemoteMachineSet) generateInstallConfigFromClusterDeployment(cd *hivev1.ClusterDeployment) (*installtypes.InstallConfig, error) {
cdLog := r.logger.WithFields(log.Fields{
"clusterDeployment": cd.Name,
Expand Down
Loading