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.

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

term: Fix confirmation prompts on windows #413

Merged
merged 1 commit into from
Nov 24, 2020

Conversation

nlowe
Copy link
Contributor

@nlowe nlowe commented Oct 28, 2020

This fixes confirmation prompts on windows by scanning for a line of input and comparing that to the desired prompt. term.Confirm(...) is refactored slightly to make it easier to test.

Fixes #347

Note that other tests (not those added in this PR) will fail on windows until #411 is merged.

I've also verified this works on windows:

With `tk` `v0.12.0`
C:\Users\Nathan\projects\grafana-agent-operator\example [master ≡ +0 ~4 -0 !]> tk apply .\environments\default\
diff -u -N C:\Users\Nathan\AppData\Local\Temp\LIVE-097035773/apiextensions.k8s.io.v1.CustomResourceDefinition..servicemonitors.monitoring.coreos.com C:\Users\Nathan\AppData\Local\Temp\MERGED-483356984/apiextensions.k8s.io.v1.CustomResourceDefinition..servicemonitors.monitoring.coreos.com
--- C:\Users\Nathan\AppData\Local\Temp\LIVE-097035773/apiextensions.k8s.io.v1.CustomResourceDefinition..servicemonitors.monitoring.coreos.com       2020-10-27 22:12:54.118744100 -0700
+++ C:\Users\Nathan\AppData\Local\Temp\MERGED-483356984/apiextensions.k8s.io.v1.CustomResourceDefinition..servicemonitors.monitoring.coreos.com     2020-10-27 22:12:54.142744000 -0700
@@ -46,7 +46,7 @@
         f:storedVersions: {}
     manager: kubectl-client-side-apply
     operation: Update
-    time: "2020-10-28T05:04:03Z"
+    time: "2020-10-28T05:12:51Z"
   name: servicemonitors.monitoring.coreos.com
   resourceVersion: "545577"
   selfLink: /apis/apiextensions.k8s.io/v1/customresourcedefinitions/servicemonitors.monitoring.coreos.com

Applying to namespace 'default' of cluster 'docker-desktop' at 'https://kubernetes.docker.internal:6443' using context 'docker-desktop'.
Please type 'yes' to confirm: yes
aborted by user
With `tk` built from this commit
C:\Users\Nathan\projects\grafana-agent-operator\example [master ≡ +0 ~4 -0 !]> ..\..\tanka\tk.exe apply .\environments\default\
diff -u -N C:\Users\Nathan\AppData\Local\Temp\LIVE-142144125/apiextensions.k8s.io.v1.CustomResourceDefinition..servicemonitors.monitoring.coreos.com C:\Users\Nathan\AppData\Local\Temp\MERGED-139668408/apiextensions.k8s.io.v1.CustomResourceDefinition..servicemonitors.monitoring.coreos.com
--- C:\Users\Nathan\AppData\Local\Temp\LIVE-142144125/apiextensions.k8s.io.v1.CustomResourceDefinition..servicemonitors.monitoring.coreos.com       2020-10-27 22:04:00.340253300 -0700
+++ C:\Users\Nathan\AppData\Local\Temp\MERGED-139668408/apiextensions.k8s.io.v1.CustomResourceDefinition..servicemonitors.monitoring.coreos.com     2020-10-27 22:04:00.366253100 -0700
@@ -46,7 +46,7 @@
         f:storedVersions: {}
     manager: kubectl-client-side-apply
     operation: Update
-    time: "2020-10-28T05:03:23Z"
+    time: "2020-10-28T05:03:57Z"
   name: servicemonitors.monitoring.coreos.com
   resourceVersion: "545509"
   selfLink: /apis/apiextensions.k8s.io/v1/customresourcedefinitions/servicemonitors.monitoring.coreos.com

Applying to namespace 'default' of cluster 'docker-desktop' at 'https://kubernetes.docker.internal:6443' using context 'docker-desktop'.
Please type 'yes' to confirm: yes
namespace/etcd unchanged
namespace/foo-a unchanged
namespace/foo-b unchanged
namespace/foo-c unchanged
namespace/monitoring unchanged
serviceaccount/grafana-agent-cluster unchanged
configmap/cortex-config unchanged
configmap/dashboards-0 unchanged
configmap/dashboards-1 unchanged
configmap/dashboards-2 unchanged
configmap/dashboards-3 unchanged
configmap/dashboards-4 unchanged
configmap/dashboards-5 unchanged
configmap/dashboards-6 unchanged
configmap/dashboards-7 unchanged
configmap/grafana-agent-cluster unchanged
configmap/grafana-config unchanged
configmap/grafana-dashboard-provisioning unchanged
configmap/grafana-datasources unchanged
persistentvolumeclaim/cortex-data unchanged
customresourcedefinition.apiextensions.k8s.io/servicemonitors.monitoring.coreos.com configured
clusterrole.rbac.authorization.k8s.io/grafana-agent-cluster unchanged
clusterrolebinding.rbac.authorization.k8s.io/grafana-agent-cluster unchanged
service/etcd unchanged
service/example unchanged
service/example unchanged
service/example unchanged
service/cortex unchanged
service/grafana unchanged
service/grafana-agent-cluster unchanged
deployment.apps/etcd unchanged
deployment.apps/example unchanged
deployment.apps/load-generator unchanged
deployment.apps/example unchanged
deployment.apps/load-generator unchanged
deployment.apps/example unchanged
deployment.apps/load-generator unchanged
deployment.apps/cortex unchanged
deployment.apps/grafana unchanged
deployment.apps/grafana-agent-cluster configured
servicemonitor.monitoring.coreos.com/example unchanged
servicemonitor.monitoring.coreos.com/example unchanged
servicemonitor.monitoring.coreos.com/example unchanged
servicemonitor.monitoring.coreos.com/cortex unchanged

This fixes confirmation prompts on windows by scanning for a line of
input and comparing that to the desired prompt. term.Confirm(...) is
refactored slightly to make it easier to test.

Fixes grafana#347
Copy link
Member

@sh0rez sh0rez left a comment

Choose a reason for hiding this comment

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

Nice work! Sorry it took so long to review. And thanks for adding a test here :)

@sh0rez sh0rez merged commit ebdc788 into grafana:master Nov 24, 2020
@nlowe nlowe deleted the bugfix/GH-347-windows-apply-confirm branch November 24, 2020 20:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Confirmation prompt not working on Windows
2 participants