diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 908f4d48..bb574046 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -151,6 +151,17 @@ jobs: pip install -r requirements.txt ONCALL_TESTING_RBAC_ENABLED=${{ matrix.rbac_enabled }} pytest -x + unit-test-pd-migrator: + runs-on: ubuntu-latest + container: python:3.9 + steps: + - uses: actions/checkout@v2 + - name: Unit Test PD Migrator + run: | + cd tools/pagerduty-migrator + pip install -r requirements.txt + pytest -x + docker-build: runs-on: ubuntu-latest steps: diff --git a/.github/workflows/helm_tests.yml b/.github/workflows/helm_tests.yml index a95b4389..658a9faf 100644 --- a/.github/workflows/helm_tests.yml +++ b/.github/workflows/helm_tests.yml @@ -10,10 +10,10 @@ jobs: - name: Checkout uses: actions/checkout@v3 - - name: Set up Docker Buildx # We need this step for docker caching + - name: Set up Docker Buildx # We need this step for docker caching uses: docker/setup-buildx-action@v2 - - name: Build docker image locally # using github actions docker cache + - name: Build docker image locally # using github actions docker cache uses: docker/build-push-action@v2 with: context: ./engine diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 9bc86f5a..5f384934 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -5,6 +5,10 @@ repos: - id: isort files: ^engine args: [--settings-file=engine/pyproject.toml, --filter-files] + - id: isort + name: isort - pd-migrator + files: ^tools/pagerduty-migrator + args: [--settings-file=tools/pagerduty-migrator/.isort.cfg, --filter-files] - repo: https://github.com/psf/black rev: 22.3.0 @@ -12,6 +16,9 @@ repos: - id: black files: ^engine args: [--config=engine/pyproject.toml] + - id: black + name: black - pd-migrator + files: ^tools/pagerduty-migrator - repo: https://github.com/pycqa/flake8 rev: 3.9.2 @@ -21,6 +28,12 @@ repos: args: [--config=engine/tox.ini] additional_dependencies: - flake8-tidy-imports + - id: flake8 + name: flake8 - pd-migrator + files: ^tools/pagerduty-migrator + # Make sure config is compatible with black + # https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html#line-length + args: [--max-line-length=88, "--select=C,E,F,W,B,B950", "--extend-ignore=E203,E501"] - repo: https://github.com/pre-commit/mirrors-eslint rev: v8.25.0 diff --git a/CHANGELOG.md b/CHANGELOG.md index 3120bf88..c6d7642b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- Added ability to change engine deployment update strategy via values in helm chart. - removed APNS support - changed the `django-push-notification` library from the `iskhakov` fork to the [`grafana` fork](https://github.com/grafana/django-push-notifications). This new fork basically patches an issue which affected the database migrations of this django app (previously the diff --git a/Makefile b/Makefile index 4ada9540..711e3e3d 100644 --- a/Makefile +++ b/Makefile @@ -86,7 +86,7 @@ restart: $(call run_docker_compose_command,restart) build: - $(call run_docker_compose_command,build --no-cache) + $(call run_docker_compose_command,build) cleanup: stop docker system prune --filter label="$(DOCKER_COMPOSE_DEV_LABEL)" --all --volumes diff --git a/dev/README.md b/dev/README.md index 33f18380..19d3ddd1 100644 --- a/dev/README.md +++ b/dev/README.md @@ -13,6 +13,7 @@ - [Could not build wheels for cryptography which use PEP 517 and cannot be installed directly](#could-not-build-wheels-for-cryptography-which-use-pep-517-and-cannot-be-installed-directly) - [django.db.utils.OperationalError: (1366, "Incorrect string value")](#djangodbutilsoperationalerror-1366-incorrect-string-value) - [/bin/sh: line 0: cd: grafana-plugin: No such file or directory](#binsh-line-0-cd-grafana-plugin-no-such-file-or-directory) + - [Encountered error while trying to install package - grpcio](#encountered-error-while-trying-to-install-package---grpcio) - [IDE Specific Instructions](#ide-specific-instructions) - [PyCharm](#pycharm) @@ -289,6 +290,34 @@ clear everything in docker by resetting or: make cleanup ``` +### Encountered error while trying to install package - grpcio + +**Problem:** + +We are currently using a library, `fcm-django`, which has a dependency on `grpcio`. Google does not provide `grpcio` +wheels built for Apple Silicon Macs. The best solution so far has been to use a `conda` virtualenv. There's apparently +a lot of community work put into making packages play well with M1/arm64 architecture. + +```bash +pip install -r requirements.txt +... + note: This error originates from a subprocess, and is likely not a problem with pip. +error: legacy-install-failure + +× Encountered error while trying to install package. +╰─> grpcio +... +``` + +**Solution:** + +Use a `conda` virtualenv, and then run the following when installing the engine dependencies/ +[See here for more details](https://stackoverflow.com/a/74307636/3902555) + +```bash +GRPC_PYTHON_BUILD_SYSTEM_OPENSSL=1 GRPC_PYTHON_BUILD_SYSTEM_ZLIB=1 pip install -r requirements.txt +``` + ## IDE Specific Instructions ### PyCharm diff --git a/docker-compose-developer.yml b/docker-compose-developer.yml index 8f00e0f2..630c1e5a 100644 --- a/docker-compose-developer.yml +++ b/docker-compose-developer.yml @@ -24,6 +24,7 @@ x-env-files: &oncall-env-files x-env-vars: &oncall-env-vars BROKER_TYPE: ${BROKER_TYPE} GRAFANA_API_URL: http://localhost:3000 + GOOGLE_APPLICATION_CREDENTIALS: /etc/app/gcp_service_account.json # basically this is needed because the oncall backend containers have been configured to communicate w/ grafana via # http://localhost:3000 (GRAFANA_API_URL). This URL is used in two scenarios: @@ -47,6 +48,7 @@ services: labels: *oncall-labels environment: ONCALL_API_URL: http://host.docker.internal:8080 + MOBILE_APP_QR_INTERVAL_QUEUE: 290000 # 4 minutes and 50 seconds volumes: - ./grafana-plugin:/etc/app - /etc/app/node_modules @@ -105,6 +107,21 @@ services: profiles: - engine + flower: + container_name: flower + labels: *oncall-labels + image: mher/flower:1.2.0 + environment: + # TODO: make this work properly w/ BROKER_TYPE env var + CELERY_BROKER_URL: "redis://redis:6379/0" + ports: + - "5555:5555" + depends_on: + oncall_celery: + condition: service_started + profiles: + - engine + oncall_db_migration: container_name: oncall_db_migration labels: *oncall-labels diff --git a/engine/.gitignore b/engine/.gitignore index 04b9e080..43962828 100644 --- a/engine/.gitignore +++ b/engine/.gitignore @@ -3,3 +3,4 @@ extensions/ uwsgi-local.ini celerybeat-schedule *.db +gcp_service_account.json diff --git a/engine/Dockerfile b/engine/Dockerfile index 4c84a01c..72046a3c 100644 --- a/engine/Dockerfile +++ b/engine/Dockerfile @@ -1,8 +1,15 @@ -FROM python:3.9-alpine3.16 AS base -RUN apk add bash python3-dev build-base linux-headers pcre-dev mariadb-connector-c-dev openssl-dev libffi-dev git +FROM python:3.9-slim-buster AS base +RUN apt-get update && apt-get install -y \ + python3-dev \ + gcc \ + libmariadb-dev \ + netcat \ + curl \ + bash WORKDIR /etc/app COPY ./requirements.txt ./ +RUN pip install --upgrade pip RUN pip install -r requirements.txt # we intentionally have two COPY commands, this is to have the requirements.txt in a separate build step @@ -16,7 +23,7 @@ RUN DJANGO_SETTINGS_MODULE=settings.prod_without_db DATABASE_TYPE=sqlite3 DATABA RUN chown -R 1000:2000 /var/lib/oncall FROM base AS dev -RUN apk add sqlite mysql-client postgresql-client +RUN apt-get install -y sqlite3 default-mysql-client postgresql-client FROM dev AS dev-enterprise RUN pip install -r requirements-enterprise-docker.txt diff --git a/engine/apps/alerts/models/alert_group.py b/engine/apps/alerts/models/alert_group.py index 8a00fb83..e201d660 100644 --- a/engine/apps/alerts/models/alert_group.py +++ b/engine/apps/alerts/models/alert_group.py @@ -93,7 +93,7 @@ class AlertGroupQuerySet(models.QuerySet): ) except IntegrityError: try: - return self.get(**search_params, is_open_for_grouping=True), False + return self.get(**search_params, is_open_for_grouping__isnull=False), False except self.model.DoesNotExist: pass raise diff --git a/engine/apps/mobile_app/backend.py b/engine/apps/mobile_app/backend.py index db518de0..43f257c3 100644 --- a/engine/apps/mobile_app/backend.py +++ b/engine/apps/mobile_app/backend.py @@ -1,7 +1,7 @@ import json from django.conf import settings -from push_notifications.models import GCMDevice +from fcm_django.models import FCMDevice from apps.base.messaging import BaseMessagingBackend from apps.mobile_app.tasks import notify_user_async @@ -35,7 +35,7 @@ class MobileAppBackend(BaseMessagingBackend): token.delete() # delete push notification related info for user - GCMDevice.objects.filter(user=user).delete() + FCMDevice.objects.filter(user=user).delete() def serialize_user(self, user): from apps.mobile_app.models import MobileAppAuthToken diff --git a/engine/apps/mobile_app/fcm_relay.py b/engine/apps/mobile_app/fcm_relay.py index 33bebda4..f50f2434 100644 --- a/engine/apps/mobile_app/fcm_relay.py +++ b/engine/apps/mobile_app/fcm_relay.py @@ -1,4 +1,5 @@ -from push_notifications.gcm import send_message +# from firebase_admin.messaging import Message +# from fcm_django.models import FCMDevice from rest_framework import status from rest_framework.response import Response from rest_framework.views import APIView @@ -6,6 +7,7 @@ from rest_framework.views import APIView REQUIRED_FIELDS = {"registration_ids", "notification", "data"} +# TODO: update thie class FCMRelayView(APIView): def post(self, request): """ @@ -17,10 +19,11 @@ class FCMRelayView(APIView): if not REQUIRED_FIELDS.issubset(request.data.keys()): return Response(status=status.HTTP_400_BAD_REQUEST) - registration_ids = request.data["registration_ids"] - data = { - **request.data["data"], - **request.data["notification"], - } + # registration_ids = request.data["registration_ids"] + # data = { + # **request.data["data"], + # **request.data["notification"], + # } - return send_message(registration_ids=registration_ids, data=data, cloud_type="FCM") + # return FCMDevice.objects.send_message(Message(), False, ["registration_ids"]) + return "TODO:" diff --git a/engine/apps/mobile_app/models.py b/engine/apps/mobile_app/models.py index e42ade25..7ad107dd 100644 --- a/engine/apps/mobile_app/models.py +++ b/engine/apps/mobile_app/models.py @@ -1,5 +1,6 @@ from typing import Tuple +from django.conf import settings from django.db import models from django.utils import timezone @@ -7,7 +8,7 @@ from apps.auth_token import constants, crypto from apps.auth_token.models import BaseAuthToken from apps.user_management.models import Organization, User -MOBILE_APP_AUTH_VERIFICATION_TOKEN_TIMEOUT_SECONDS = 60 +MOBILE_APP_AUTH_VERIFICATION_TOKEN_TIMEOUT_SECONDS = 60 * (5 if settings.DEBUG else 1) def get_expire_date(): diff --git a/engine/apps/mobile_app/tasks.py b/engine/apps/mobile_app/tasks.py index fc9341d9..88d56a17 100644 --- a/engine/apps/mobile_app/tasks.py +++ b/engine/apps/mobile_app/tasks.py @@ -1,6 +1,7 @@ from celery.utils.log import get_task_logger from django.conf import settings -from push_notifications.models import GCMDevice +from fcm_django.models import FCMDevice +from firebase_admin.messaging import APNSConfig, APNSPayload, Aps, ApsAlert, CriticalSound, Message from apps.alerts.models import AlertGroup from apps.mobile_app.alert_rendering import get_push_notification_message @@ -34,10 +35,10 @@ def notify_user_async(user_pk, alert_group_pk, notification_policy_pk, critical) logger.warning(f"User notification policy {notification_policy_pk} does not exist") return - gcm_devices_to_notify = GCMDevice.objects.filter(user=user) + device_to_notify = FCMDevice.objects.filter(user=user).first() # create an error log in case user has no devices set up - if not gcm_devices_to_notify.exists(): + if not device_to_notify: UserNotificationPolicyLogRecord.objects.create( author=user, type=UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_FAILED, @@ -47,39 +48,58 @@ def notify_user_async(user_pk, alert_group_pk, notification_policy_pk, critical) notification_step=notification_policy.step, notification_channel=notification_policy.notify_by, ) - logger.info(f"Error while sending a mobile push notification: user {user_pk} has no devices set up") + logger.info(f"Error while sending a mobile push notification: user {user_pk} has no device set up") return message = get_push_notification_message(alert_group) thread_id = f"{alert_group.channel.organization.public_primary_key}:{alert_group.public_primary_key}" + alert_title = f"Critical page: {message}" if critical else message + number_of_alerts = alert_group.alerts.count() - if critical: - aps = { - "alert": f"Critical page: {message}", - "interruption-level": "critical", - "sound": "ambulance.aiff", - } - else: - aps = { - "alert": message, - "sound": "bingbong.aiff", - } + # TODO: we should update this to check if FCM_RELAY is set and conditionally make a call here.. - extra = { - "orgId": alert_group.channel.organization.public_primary_key, - "orgName": alert_group.channel.organization.stack_slug, - "alertGroupId": alert_group.public_primary_key, - "status": alert_group.status, - "aps": aps, - } - - logger.info(f"Sending push notification with message: {message}; thread-id: {thread_id}; extra: {extra}") - - # TODO: rename category to USER_NEW_ALERT_GROUP - fcm_response = gcm_devices_to_notify.send_message( - message, thread_id=thread_id, category="USER_NEW_INCIDENT", extra=extra + message = Message( + token=device_to_notify.registration_id, + data={ + # from the docs.. + # A dictionary of data fields (optional). All keys and values in the dictionary must be strings + # + # alert_group.status is an int so it must be casted... + "orgId": alert_group.channel.organization.public_primary_key, + "orgName": alert_group.channel.organization.stack_slug, + "alertGroupId": alert_group.public_primary_key, + "status": str(alert_group.status), + "type": "oncall.critical_message" if critical else "oncall.message", + "title": alert_title, + "thread_id": thread_id, + }, + apns=APNSConfig( + payload=APNSPayload( + aps=Aps( + thread_id=thread_id, + badge=number_of_alerts, + alert=ApsAlert( + title=alert_title, + subtitle="yooo this is a subtitle", + body="hello this is the body", + ), + sound=CriticalSound( + critical=1 if critical else 0, + name="ambulance.aiff" if critical else "bingbong.aiff", + volume=1, + ), + custom_data={ + "interruption-level": "critical" if critical else "time-sensitive", + }, + ), + ), + ), ) + logger.info(f"Sending push notification with message: {message}; thread-id: {thread_id};") + + fcm_response = device_to_notify.send_message(message) + # NOTE: we may want to further handle the response from FCM, but for now lets simply log it out # https://firebase.google.com/docs/cloud-messaging/http-server-ref#interpret-downstream logger.info(f"FCM response was: {fcm_response}") diff --git a/engine/apps/mobile_app/views.py b/engine/apps/mobile_app/views.py index b0188b3e..16257440 100644 --- a/engine/apps/mobile_app/views.py +++ b/engine/apps/mobile_app/views.py @@ -1,25 +1,15 @@ -from push_notifications.api.rest_framework import GCMDeviceAuthorizedViewSet, GCMDeviceSerializer +from fcm_django.api.rest_framework import FCMDeviceAuthorizedViewSet as BaseFCMDeviceAuthorizedViewSet from rest_framework import status from rest_framework.exceptions import NotFound from rest_framework.response import Response -from rest_framework.serializers import HiddenField from rest_framework.views import APIView from apps.mobile_app.auth import MobileAppAuthTokenAuthentication, MobileAppVerificationTokenAuthentication from apps.mobile_app.models import MobileAppAuthToken -class FCMDeviceAuthorizedViewSet(GCMDeviceAuthorizedViewSet): - class FCMDeviceSerializer(GCMDeviceSerializer): - """ - GCMDevice has cloud_message_type equal to "GCM" by default, in this serializer cloud_message_type is always set - to "FCM" no matter what was provided in the request. - """ - - cloud_message_type = HiddenField(default="FCM") - +class FCMDeviceAuthorizedViewSet(BaseFCMDeviceAuthorizedViewSet): authentication_classes = (MobileAppAuthTokenAuthentication,) - serializer_class = FCMDeviceSerializer class MobileAppAuthTokenAPIView(APIView): diff --git a/engine/apps/telegram/renderers/message.py b/engine/apps/telegram/renderers/message.py index 1e989fe4..8140c3e5 100644 --- a/engine/apps/telegram/renderers/message.py +++ b/engine/apps/telegram/renderers/message.py @@ -47,6 +47,8 @@ class TelegramMessageRenderer: message_trimmed_text = MESSAGE_TRIMMED_TEXT.format(link=self.alert_group.web_link) max_log_lines_length = max_message_length - len(start_line_text) - len(message_trimmed_text) + if max_log_lines_length < 0: + return "" is_message_trimmed = len("\n".join(log_lines)) > max_log_lines_length while len("\n".join(log_lines)) > max_log_lines_length: log_lines.pop() diff --git a/engine/requirements.txt b/engine/requirements.txt index f179c8d9..88765b9c 100644 --- a/engine/requirements.txt +++ b/engine/requirements.txt @@ -33,7 +33,7 @@ django-log-request-id==1.6.0 django-polymorphic==3.0.0 django-rest-polymorphic==0.1.9 pre-commit==2.15.0 -https://github.com/grafana/django-push-notifications/archive/refs/tags/3.0.0-fix-migration.tar.gz +https://github.com/grafana/fcm-django/archive/refs/tags/v1.0.12r1.tar.gz django-mirage-field==1.3.0 django-mysql==4.6.0 PyMySQL==1.0.2 diff --git a/engine/settings/base.py b/engine/settings/base.py index 96737486..e9abfc7b 100644 --- a/engine/settings/base.py +++ b/engine/settings/base.py @@ -219,7 +219,7 @@ INSTALLED_APPS = [ "debug_toolbar", "social_django", "polymorphic", - "push_notifications", + "fcm_django", ] REST_FRAMEWORK = { @@ -549,18 +549,26 @@ GRAFANA_API_KEY_NAME = "Grafana OnCall" EXTRA_MESSAGING_BACKENDS = [] if FEATURE_MOBILE_APP_INTEGRATION_ENABLED: + from firebase_admin import initialize_app + EXTRA_MESSAGING_BACKENDS += [ ("apps.mobile_app.backend.MobileAppBackend", 5), ("apps.mobile_app.backend.MobileAppCriticalBackend", 6), ] -PUSH_NOTIFICATIONS_SETTINGS = { - "FCM_API_KEY": os.getenv("FCM_API_KEY"), - "FCM_POST_URL": os.getenv("FCM_POST_URL", default="https://fcm.googleapis.com/fcm/send"), - "USER_MODEL": "user_management.User", - "UPDATE_ON_DUPLICATE_REG_ID": True, -} + FIREBASE_APP = initialize_app(options={"projectId": os.environ.get("FCM_PROJECT_ID", None)}) + FCM_RELAY_ENABLED = getenv_boolean("FCM_RELAY_ENABLED", default=False) +FCM_DJANGO_SETTINGS = { + # an instance of firebase_admin.App to be used as default for all fcm-django requests + # default: None (the default Firebase app) + "DEFAULT_FIREBASE_APP": None, + "APP_VERBOSE_NAME": "OnCall", + "ONE_DEVICE_PER_USER": True, + "DELETE_INACTIVE_DEVICES": False, + "UPDATE_ON_DUPLICATE_REG_ID": True, + "USER_MODEL": "user_management.User", +} SELF_HOSTED_SETTINGS = { "STACK_ID": 5, diff --git a/grafana-plugin/src/containers/MobileAppVerification/MobileAppVerification.module.scss b/grafana-plugin/src/containers/MobileAppVerification/MobileAppVerification.module.scss index cd4af8f4..94dbd47f 100644 --- a/grafana-plugin/src/containers/MobileAppVerification/MobileAppVerification.module.scss +++ b/grafana-plugin/src/containers/MobileAppVerification/MobileAppVerification.module.scss @@ -39,6 +39,11 @@ opacity: 0.2; } +.qr-code { + background-color: #fff; + margin-bottom: 12px; +} + .qr-loader { position: absolute; z-index: 10; diff --git a/grafana-plugin/src/containers/MobileAppVerification/MobileAppVerification.tsx b/grafana-plugin/src/containers/MobileAppVerification/MobileAppVerification.tsx index ed634a9f..c31604a5 100644 --- a/grafana-plugin/src/containers/MobileAppVerification/MobileAppVerification.tsx +++ b/grafana-plugin/src/containers/MobileAppVerification/MobileAppVerification.tsx @@ -22,7 +22,9 @@ type Props = { }; const INTERVAL_MIN_THROTTLING = 500; -const INTERVAL_QUEUE_QR = 50000; +const INTERVAL_QUEUE_QR = process.env.MOBILE_APP_QR_INTERVAL_QUEUE + ? parseInt(process.env.MOBILE_APP_QR_INTERVAL_QUEUE, 10) + : 50000; const INTERVAL_POLLING = 5000; const BACKEND = 'MOBILE_APP'; @@ -133,7 +135,7 @@ const MobileAppVerification = observer(({ userPk }: Props) => { Open Grafana IRM mobile application and scan this code to sync it with your account.
- + {isQRBlurry && }
diff --git a/grafana-plugin/src/containers/MobileAppVerification/__snapshots__/MobileAppVerification.test.tsx.snap b/grafana-plugin/src/containers/MobileAppVerification/__snapshots__/MobileAppVerification.test.tsx.snap index e471869c..ac791478 100644 --- a/grafana-plugin/src/containers/MobileAppVerification/__snapshots__/MobileAppVerification.test.tsx.snap +++ b/grafana-plugin/src/containers/MobileAppVerification/__snapshots__/MobileAppVerification.test.tsx.snap @@ -109,7 +109,7 @@ exports[`MobileAppVerification if we disconnect the app, it disconnects and fetc class="u-width-100 u-flex u-flex-center u-position-relative" >
{ */ new webpack.EnvironmentPlugin({ ONCALL_API_URL: null, + MOBILE_APP_QR_INTERVAL_QUEUE: null, }), ], diff --git a/helm/oncall/Chart.yaml b/helm/oncall/Chart.yaml index 70f7df3c..76ffef88 100644 --- a/helm/oncall/Chart.yaml +++ b/helm/oncall/Chart.yaml @@ -7,7 +7,7 @@ type: application # This is the chart version. This version number should be incremented each time you make changes # to the chart and its templates, including the app version. # Versions are expected to follow Semantic Versioning (https://semver.org/) -version: 1.1.0 +version: 1.1.1 # This is the version number of the application being deployed. This version number should be # incremented each time you make changes to the application. Versions are not expected to diff --git a/helm/oncall/templates/engine/deployment.yaml b/helm/oncall/templates/engine/deployment.yaml index 640bb84b..f0848c76 100644 --- a/helm/oncall/templates/engine/deployment.yaml +++ b/helm/oncall/templates/engine/deployment.yaml @@ -10,10 +10,7 @@ spec: matchLabels: {{- include "oncall.engine.selectorLabels" . | nindent 6 }} strategy: - rollingUpdate: - maxSurge: 25% - maxUnavailable: 0 - type: RollingUpdate + {{- toYaml .Values.engine.updateStrategy | nindent 4 }} template: metadata: {{- with .Values.podAnnotations }} diff --git a/helm/oncall/values.yaml b/helm/oncall/values.yaml index a21ce82c..9abb14fc 100644 --- a/helm/oncall/values.yaml +++ b/helm/oncall/values.yaml @@ -29,6 +29,14 @@ engine: # cpu: 100m # memory: 128Mi + ## Deployment update strategy + ## ref: https://kubernetes.io/docs/concepts/workloads/controllers/deployment/#strategy + updateStrategy: + rollingUpdate: + maxSurge: 25% + maxUnavailable: 0 + type: RollingUpdate + ## Affinity for pod assignment ## ref: https://kubernetes.io/docs/concepts/configuration/assign-pod-node/#affinity-and-anti-affinity affinity: {} diff --git a/tools/pagerduty-migrator/migrator/report.py b/tools/pagerduty-migrator/migrator/report.py index 39c769ca..4947a44f 100644 --- a/tools/pagerduty-migrator/migrator/report.py +++ b/tools/pagerduty-migrator/migrator/report.py @@ -105,13 +105,13 @@ def escalation_policy_report(policies: list[dict]) -> str: for policy in sorted( policies, key=lambda p: bool(p["unmatched_users"] or p["flawed_schedules"]) ): - result += f"\n" + TAB + format_escalation_policy(policy) + result += "\n" + TAB + format_escalation_policy(policy) for user in policy["unmatched_users"]: - result += f"\n" + TAB * 2 + format_user(user) + result += "\n" + TAB * 2 + format_user(user) for schedule in policy["flawed_schedules"]: - result += f"\n" + TAB * 2 + format_schedule(schedule) + result += "\n" + TAB * 2 + format_schedule(schedule) if ( not policy["unmatched_users"] @@ -135,7 +135,7 @@ def integration_report(integrations: list[dict]) -> str: key=lambda i: bool(i["oncall_type"] and not i["is_escalation_policy_flawed"]), reverse=True, ): - result += f"\n" + TAB + format_integration(integration) + result += "\n" + TAB + format_integration(integration) if ( integration["oncall_type"] and not integration["is_escalation_policy_flawed"] diff --git a/tools/pagerduty-migrator/migrator/resources/users.py b/tools/pagerduty-migrator/migrator/resources/users.py index e616d52c..ba9008f2 100644 --- a/tools/pagerduty-migrator/migrator/resources/users.py +++ b/tools/pagerduty-migrator/migrator/resources/users.py @@ -4,7 +4,7 @@ from migrator.utils import find_by_id def match_user(user: dict, oncall_users: list[dict]) -> None: oncall_user = None for candidate_user in oncall_users: - if user["email"] == candidate_user["email"]: + if user["email"].lower() == candidate_user["email"].lower(): oncall_user = candidate_user break diff --git a/tools/pagerduty-migrator/migrator/tests/test_matching.py b/tools/pagerduty-migrator/migrator/tests/test_matching.py index 4f74e003..556b3782 100644 --- a/tools/pagerduty-migrator/migrator/tests/test_matching.py +++ b/tools/pagerduty-migrator/migrator/tests/test_matching.py @@ -1553,21 +1553,14 @@ expected_integrations_result = [ ] -def test_matching(): - match_users_test() - match_schedule_test() - match_escalation_policies_test() - match_integrations_test() - - -def match_users_test(): +def test_match_user(): for user in pd_users_payload: match_user(user, oncall_users_payload) assert pd_users_payload == expected_users_match_result -def match_schedule_test(): +def test_match_schedule(): for schedule in pd_schedules_payload: match_schedule(schedule, oncall_schedules_payload) match_users_for_schedule(schedule, pd_users_payload) @@ -1575,7 +1568,7 @@ def match_schedule_test(): assert pd_schedules_payload == expected_schedules_result -def match_escalation_policies_test(): +def test_match_escalation_policy(): for policy in pd_escalation_policies_payload: match_escalation_policy(policy, oncall_escalation_chains) match_users_and_schedules_for_escalation_policy( @@ -1585,7 +1578,7 @@ def match_escalation_policies_test(): assert pd_escalation_policies_payload == expected_escalation_policies_result -def match_integrations_test(): +def test_match_integration(): integrations = [] for service in pd_services_payload: service_integrations = service.pop("integrations") diff --git a/tools/pagerduty-migrator/migrator/tests/test_user_matching.py b/tools/pagerduty-migrator/migrator/tests/test_user_matching.py new file mode 100644 index 00000000..539ab693 --- /dev/null +++ b/tools/pagerduty-migrator/migrator/tests/test_user_matching.py @@ -0,0 +1,17 @@ +from migrator.resources.users import match_user + + +def test_match_user_not_found(): + pd_user = {"email": "test@test.com"} + oncall_users = [{"email": "test1@test.com"}] + + match_user(pd_user, oncall_users) + assert pd_user["oncall_user"] is None + + +def test_match_user_email_case_insensitive(): + pd_user = {"email": "test@test.com"} + oncall_users = [{"email": "TEST@TEST.COM"}] + + match_user(pd_user, oncall_users) + assert pd_user["oncall_user"] == oncall_users[0] diff --git a/tools/pagerduty-migrator/requirements.txt b/tools/pagerduty-migrator/requirements.txt index 339e1095..8e7340c8 100644 --- a/tools/pagerduty-migrator/requirements.txt +++ b/tools/pagerduty-migrator/requirements.txt @@ -1,6 +1,4 @@ requests==2.27.1 pdpyras==4.5.0 -isort==5.10.1 -black==22.3.0 pytest==7.1.2 pytest-env==0.6.2 \ No newline at end of file