Skip to content

Conversation

derekwaynecarr
Copy link
Member

This adds pretty printers when rendering tables in CLI.

Machines

oc get machines -n openshift-cluster-api
NAME                                 INSTANCE              STATE     TYPE       REGION      ZONE         AGE
decarr-dev-master-0                  i-0c5c6028992720b04   running   m4.large   us-east-2   us-east-2a   4h

MachineSets

oc get machinesets -n openshift-cluster-api
NAME                           DESIRED   CURRENT   READY     AVAILABLE   AGE
decarr-dev-worker-us-east-2a   1         1         1         1           4h

@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jan 12, 2019
@derekwaynecarr
Copy link
Member Author

It is inevitable that you will want printers for each provider.

The beta is targeting AWS, so this is good for now. As we surface other providers, we may want to look at having the machine-api-operator itself install the CRDs that vary by provider so we can have pretty columns rendered with platform specific metadata.

We should do the same for machinedeployments.

When we support machine class, we want to revisit this.

/cc @enxebre @smarterclayton

@smarterclayton
Copy link
Contributor

I hate machinesets columns (which are really deployment columns) but it's fine. I have open bugs upstream to make them not suck, but it's not your problem

@openshift-ci-robot openshift-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 14, 2019
@enxebre
Copy link
Member

enxebre commented Jan 14, 2019

/approve

@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: enxebre

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 14, 2019
@enxebre
Copy link
Member

enxebre commented Jan 14, 2019

thanks! I added a commit on your branch to make yaml lint happy

@enxebre
Copy link
Member

enxebre commented Jan 14, 2019

/test e2e-aws

@ingvagabund
Copy link
Member

ingvagabund commented Jan 14, 2019

It is inevitable that you will want printers for each provider.

Would be great to have a clean way to do that.

The beta is targeting AWS, so this is good for now. As we surface other providers, we may want to look at having the machine-api-operator itself install the CRDs that vary by provider so we can have pretty columns rendered with platform specific metadata.

As long as the CRDs are again managed by the machine-api-operator, it is acceptable. Though, it will cause troubles when (and if) we start supporting multiple instances of an actuator inside the same cluster (a.k.a hybrid clouds).

@ingvagabund
Copy link
Member

/lgtm

/retest

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 14, 2019
@ingvagabund
Copy link
Member

/retest

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@ingvagabund
Copy link
Member

/retest

@openshift-merge-robot openshift-merge-robot merged commit cc5b35e into openshift:master Jan 14, 2019
ingvagabund pushed a commit to ingvagabund/machine-api-operator that referenced this pull request Jul 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants