-
Notifications
You must be signed in to change notification settings - Fork 246
FLINK-5725: Add extra Flink details to paasta status #4063
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
Changes from 6 commits
8ec0c76
fab0eb5
28381c5
1fb40c6
56cdcc5
02c4363
7806ecf
ba32de6
2a65cc6
fa93667
16a304f
06319d8
7357296
024fdcc
1718cb1
ce4de9f
141a8ae
d78be12
2720594
d42bc6e
c48c05b
379592a
8f16b33
4cf48ba
1e89fd5
68b42b6
da0226f
d7ac273
994e4d6
33b03fa
022b9c4
ffab533
42fc583
571a4cd
377584d
22d2921
dbc5e66
e0388bb
4a93fdf
7138c66
399c0ad
b71546c
d5e6765
5bf589d
d946ba3
c1792da
4ba30b3
55b4371
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -181,6 +181,16 @@ | |||||
"SETFCAP", | ||||||
] | ||||||
|
||||||
# https://github.yelpcorp.com/sysgit/srv-configs/tree/master/superregion | ||||||
SUPPERREGION_TO_ECOSYSTEM_MAPPINGS = { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The constant name 'SUPPERREGION_TO_ECOSYSTEM_MAPPINGS' appears to have a misspelling; consider renaming it to 'SUPERREGION_TO_ECOSYSTEM_MAPPINGS' for clarity.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||
"norcal-devc": "devc", | ||||||
"norcal-stagef": "stagef", | ||||||
"norcal-stageg": "stageg", | ||||||
"nova-prod": "prod", | ||||||
"pnw-devc": "devc", | ||||||
"pnw-prod": "prod", | ||||||
} | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this should probably be accessed through SystemPaastaConfig - i.e., live in puppet There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also, this approach will still fail for infrastage - not all clusters are named after a superregion There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @nemacysts we only have 1 service/instance in infrastage that we use for testing Infrastage would require a bit of extra logic,(srv and soa links don't follow pattern) so I decided to remove it I am not sure what I would be accessing via SystemPaastaConfig |
||||||
|
||||||
|
||||||
class RollbackTypes(Enum): | ||||||
AUTOMATIC_SLO_ROLLBACK = "automatic_slo_rollback" | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2702,7 +2702,7 @@ def test_output_stopping_jobmanager( | |
output = [] | ||
mock_flink_status["status"]["state"] = "Stoppingjobmanager" | ||
print_flink_status( | ||
cluster="fake_cluster", | ||
cluster="pnw-devc", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. once the mapping is moved to SPC, we can probably mock the getter and have a fake ecosystem for fake_cluster There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @nleigh we still wanna do this: using a non-existent cluster ensures that if folks make a mistake wrt their mocks, we never actually hit a real paasta api i guess for now this is still technically safe since we don't have any service called There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see thanks |
||
service="fake_service", | ||
instance="fake_instance", | ||
output=output, | ||
|
@@ -2712,6 +2712,19 @@ def test_output_stopping_jobmanager( | |
status = mock_flink_status["status"] | ||
expected_output = [ | ||
f" Config SHA: 00000", | ||
f" Repo(git): https://github.yelpcorp.com/services/fake_service", | ||
f" Repo(sourcegraph): https://sourcegraph.yelpcorp.com/services/fake_service", | ||
f" Yelpsoa configs: https://github.yelpcorp.com/sysgit/yelpsoa-configs/tree/master/fake_service", | ||
f" Srv configs: https://github.yelpcorp.com/sysgit/srv-configs/tree/master/ecosystem/devc/fake_service", | ||
f" Flink Log Commands:", | ||
f" Service: paasta logs -a 1h -c pnw-devc -s fake_service -i fake_instance", | ||
f" Taskmanager: paasta logs -a 1h -c pnw-devc -s fake_service -i fake_instance.TASKMANAGER", | ||
f" Jobmanager: paasta logs -a 1h -c pnw-devc -s fake_service -i fake_instance.JOBMANAGER", | ||
f" Supervisor: paasta logs -a 1h -c pnw-devc -s fake_service -i fake_instance.SUPERVISOR", | ||
f" Flink Monitoring:", | ||
f" Job Metrics: https://grafana.yelpcorp.com/d/flink-metrics/flink-job-metrics?orgId=1&var-datasource=Prometheus-flink&var-region=uswest2-devc&var-service=fake_service&var-instance=fake_instance&var-job=All&from=now-24h&to=now", | ||
f" Container Metrics: https://grafana.yelpcorp.com/d/flink-container-metrics/flink-container-metrics?orgId=1&var-datasource=Prometheus-flink&var-region=uswest2-devc&var-service=fake_service&var-instance=fake_instance&from=now-24h&to=now", | ||
f" JVM Metrics: https://grafana.yelpcorp.com/d/flink-jvm-metrics/flink-jvm-metrics?orgId=1&var-datasource=Prometheus-flink&var-region=uswest2-devc&var-service=fake_service&var-instance=fake_instance&from=now-24h&to=now", | ||
f" State: {PaastaColors.yellow(status['state'].title())}", | ||
f" Pods: 3 running, 0 evicted, 0 other", | ||
] | ||
|
@@ -2745,7 +2758,7 @@ def test_output_stopping_taskmanagers( | |
"pod_status" | ||
][2:] | ||
print_flink_status( | ||
cluster="fake_cluster", | ||
cluster="pnw-devc", | ||
service="fake_service", | ||
instance="fake_instance", | ||
output=output, | ||
|
@@ -2755,6 +2768,19 @@ def test_output_stopping_taskmanagers( | |
status = mock_flink_status["status"] | ||
expected_output = [ | ||
f" Config SHA: 00000", | ||
f" Repo(git): https://github.yelpcorp.com/services/fake_service", | ||
f" Repo(sourcegraph): https://sourcegraph.yelpcorp.com/services/fake_service", | ||
f" Yelpsoa configs: https://github.yelpcorp.com/sysgit/yelpsoa-configs/tree/master/fake_service", | ||
f" Srv configs: https://github.yelpcorp.com/sysgit/srv-configs/tree/master/ecosystem/devc/fake_service", | ||
f" Flink Log Commands:", | ||
f" Service: paasta logs -a 1h -c pnw-devc -s fake_service -i fake_instance", | ||
f" Taskmanager: paasta logs -a 1h -c pnw-devc -s fake_service -i fake_instance.TASKMANAGER", | ||
f" Jobmanager: paasta logs -a 1h -c pnw-devc -s fake_service -i fake_instance.JOBMANAGER", | ||
f" Supervisor: paasta logs -a 1h -c pnw-devc -s fake_service -i fake_instance.SUPERVISOR", | ||
f" Flink Monitoring:", | ||
f" Job Metrics: https://grafana.yelpcorp.com/d/flink-metrics/flink-job-metrics?orgId=1&var-datasource=Prometheus-flink&var-region=uswest2-devc&var-service=fake_service&var-instance=fake_instance&var-job=All&from=now-24h&to=now", | ||
f" Container Metrics: https://grafana.yelpcorp.com/d/flink-container-metrics/flink-container-metrics?orgId=1&var-datasource=Prometheus-flink&var-region=uswest2-devc&var-service=fake_service&var-instance=fake_instance&from=now-24h&to=now", | ||
f" JVM Metrics: https://grafana.yelpcorp.com/d/flink-jvm-metrics/flink-jvm-metrics?orgId=1&var-datasource=Prometheus-flink&var-region=uswest2-devc&var-service=fake_service&var-instance=fake_instance&from=now-24h&to=now", | ||
f" State: {PaastaColors.yellow(status['state'].title())}", | ||
f" Pods: 1 running, 0 evicted, 0 other", | ||
] | ||
|
@@ -2784,7 +2810,7 @@ def test_output_1_verbose( | |
mock_naturaltime.return_value = "one day ago" | ||
output = [] | ||
print_flink_status( | ||
cluster="fake_cluster", | ||
cluster="pnw-devc", | ||
service="fake_service", | ||
instance="fake_instance", | ||
output=output, | ||
|
@@ -2798,6 +2824,17 @@ def test_output_1_verbose( | |
datetime.datetime.fromtimestamp(int(job_details_obj.start_time) // 1000) | ||
) | ||
expected_output = _get_base_status_verbose_1(metadata) + [ | ||
f" Yelpsoa configs: https://github.yelpcorp.com/sysgit/yelpsoa-configs/tree/master/fake_service", | ||
f" Srv configs: https://github.yelpcorp.com/sysgit/srv-configs/tree/master/ecosystem/devc/fake_service", | ||
f" Flink Log Commands:", | ||
f" Service: paasta logs -a 1h -c pnw-devc -s fake_service -i fake_instance", | ||
f" Taskmanager: paasta logs -a 1h -c pnw-devc -s fake_service -i fake_instance.TASKMANAGER", | ||
f" Jobmanager: paasta logs -a 1h -c pnw-devc -s fake_service -i fake_instance.JOBMANAGER", | ||
f" Supervisor: paasta logs -a 1h -c pnw-devc -s fake_service -i fake_instance.SUPERVISOR", | ||
f" Flink Monitoring:", | ||
f" Job Metrics: https://grafana.yelpcorp.com/d/flink-metrics/flink-job-metrics?orgId=1&var-datasource=Prometheus-flink&var-region=uswest2-devc&var-service=fake_service&var-instance=fake_instance&var-job=All&from=now-24h&to=now", | ||
f" Container Metrics: https://grafana.yelpcorp.com/d/flink-container-metrics/flink-container-metrics?orgId=1&var-datasource=Prometheus-flink&var-region=uswest2-devc&var-service=fake_service&var-instance=fake_instance&from=now-24h&to=now", | ||
f" JVM Metrics: https://grafana.yelpcorp.com/d/flink-jvm-metrics/flink-jvm-metrics?orgId=1&var-datasource=Prometheus-flink&var-region=uswest2-devc&var-service=fake_service&var-instance=fake_instance&from=now-24h&to=now", | ||
f" State: {PaastaColors.green(status['state'].title())}", | ||
f" Pods: 3 running, 0 evicted, 0 other", | ||
f" Jobs: 1 running, 0 finished, 0 failed, 0 cancelled", | ||
|
@@ -2859,6 +2896,8 @@ def _get_base_status_verbose_0(metadata): | |
def _get_base_status_verbose_1(metadata): | ||
return [ | ||
f" Config SHA: 00000", | ||
f" Repo(git): https://github.yelpcorp.com/services/fake_service", | ||
f" Repo(sourcegraph): https://sourcegraph.yelpcorp.com/services/fake_service", | ||
f" Flink version: {config_obj.flink_version} {config_obj.flink_revision}", | ||
f" URL: {metadata['annotations']['flink.yelp.com/dashboard_url']}/", | ||
] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we have a bunch of
if verbose:
blocks in a row - should these be a single block?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possibly, it was more for just formatting/separating
Tbf it's getting to the point where could maybe refactor whole function, will consider it in a new PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can separate with comments inside a single
if verbose:
block :)that said ++ to more refactoring in another PR