feat: modernization and simplification#97
Draft
butler54 wants to merge 73 commits into
Draft
Conversation
Switch from community golang-external-secrets to Red Hat's openshift-external-secrets chart across all 5 topology values files. Changes: namespace golang-external-secrets → external-secrets, chart name golang-external-secrets → openshift-external-secrets. Application key remains secrets-operator. ChartVersion stays 0.1.*. Rationale: Per validated patterns blog, Red Hat's downstream ESO chart provides better OCP integration and support. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Introduce global.clusterVersion variable (default: "4.21") to support OCP version-specific operator channel pins. Add clusterVersion to sharedValueFiles in all 5 topology files. Create per-version override files (values-4.19.yaml, values-4.20.yaml, values-4.21.yaml) containing LVMS channel pins. Rationale: LVMS operator channels are version-coupled (stable-4.19, stable-4.20, stable-4.21). This makes upgrading to new OCP versions a simple global variable change instead of editing multiple topology files. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove hardcoded 'channel: stable-4.20' from lvm-operator subscriptions in values-baremetal.yaml and values-baremetal-gpu.yaml. The channel is now set dynamically via per-version override files (values-4.19.yaml, values-4.20.yaml, values-4.21.yaml) based on global.clusterVersion. Rationale: Eliminates manual edits across topology files when upgrading OCP versions. Channel selection is now centralized in version overrides. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace hardcoded 'v4.20' NFD operand image tag with Helm template
'v{{ .Values.global.clusterVersion }}' in nfd-instance.yaml. The NFD
image tag now tracks the global.clusterVersion variable automatically.
Rationale: NFD operand images are OCP version-coupled (v4.19, v4.20,
v4.21). Templatizing eliminates manual image tag updates when upgrading
OCP versions.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add 'singleArgoCD: true' under the main: section in values-global.yaml. This consolidates all clusterGroup applications into a single ArgoCD instance instead of multiple instances per clusterGroup. Rationale: Per VP operator v0.0.76+, singleArgoCD mode reduces resource overhead and simplifies ArgoCD management for patterns with multiple clusterGroups. Flag is framework-level under main:, not pattern-level under global:. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Switch trustee and sandbox-policies applications from Helm registry references (chart + chartVersion) to git-based references (repoURL + targetRevision + path) for Phase 1 testing. Points to dev/phase1-testing branches on butler54 chart forks. Temporary change - revert to chart/chartVersion before upstream PRs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… version ESO migration from golang-external-secrets (Helm) to openshift-external-secrets (OLM): - Add external-secrets-operator namespace with operatorGroup - Add openshift-external-secrets-operator subscription (stable-v1 channel) - Fix chart version from 0.1.* to 0.0.* to match validated patterns common Changes apply to all deployment profiles: - values-baremetal.yaml - values-baremetal-gpu.yaml - values-simple.yaml - values-spoke.yaml - values-trusted-hub.yaml Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…race condition - Add argocd.argoproj.io/sync-wave: "10" to all CoCo workload deployments - hello-openshift secure and insecure-policy deployments - kbs-access secure deployment - gpu-workload deployment - Add global.coco.secured: "true" override to trustee app in values-simple.yaml and values-trusted-hub.yaml This ensures workload pods deploy after Kyverno policies and initdata ConfigMaps are created, preventing the race condition where pods start before initdata injection is ready. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ner dependency Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… topologies Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ed registry Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Complete authenticated registry support by adding the push-pull-secret imperative job to remaining topology values files. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…in trustee chart Removes ansible playbook and job references for push-pull-secret-to-kbs. The pull secret is now propagated declaratively via ACM ConfigurationPolicy in the trustee-chart (sync-wave 5), continuously reconciled by ACM. Files changed: - Deleted ansible/push-pull-secret-to-kbs.yaml - Removed job from all topology values files Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Update internal clusterGroup name from 'simple' to 'azure' to better reflect the Azure platform-specific deployment topology. This is the single-cluster configuration for Azure deployments with peer pods. The file rename preserves git history via git mv. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Update internal clusterGroup name from 'spoke' to 'azure-spoke' to clarify this is the Azure-specific spoke topology in a multi-cluster deployment. Paired with the trusted-hub configuration. The file rename preserves git history via git mv. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Update all references from 'simple' to 'azure' and 'spoke' to 'azure-spoke' across values files, CI workflows, and documentation. Changes: - values-global.yaml: update clusterGroupName comment with topology catalog - validate-defaults.yaml: CI check now validates 'azure' (upstream default) - values-trusted-hub.yaml: managedClusterGroups updated to azure-spoke - README.md: update all topology references and deployment instructions - AGENTS.md: update topology table and file tree listings No functional changes — only naming consistency updates. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add hardware.profile configuration to values-global.yaml for gating hardware-specific operators (GPU, Intel device plugins, DCAP) in the baremetal topology. Supports four profiles: intel-tdx, amd-snp, intel-tdx-gpu, amd-snp-gpu. Defaults to intel-tdx to match current test cluster hardware. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Merge all GPU content from values-baremetal-gpu.yaml into values- baremetal.yaml. Hardware profile overrides will gate which operators and applications are active. Changes: - Add GPU namespaces (nvidia-gpu-operator, gpu-workload) - Add gpu-operator subscription (certified, v26.3) - Add nvidia-gpu and gpu-workload applications - Add kbs.gpu.enabled trustee override - Add reconcile-kataconfig-gpu imperative job - Add hardware profile to sharedValueFiles as last entry - Update header comment to mention hardware profiles All components are enabled by default. Override files will set disabled: true for inapplicable hardware configurations. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add four hardware profile override files to gate operators and applications based on hardware configuration: - intel-tdx: disable GPU components (Intel TDX only) - amd-snp: disable Intel and GPU components (AMD SEV-SNP only) - intel-tdx-gpu: enable all (Intel TDX + NVIDIA GPU) - amd-snp-gpu: disable Intel, enable GPU (AMD SEV-SNP + NVIDIA GPU) Override files use the VP framework's native disabled: true support. Loaded via sharedValueFiles in values-baremetal.yaml. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove values-baremetal-gpu.yaml now that all GPU content has been merged into values-baremetal.yaml. Hardware profile system replaces the separate topology file approach. Users should now set global.hardware.profile instead of using baremetal-gpu as a separate clusterGroupName. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Update README.md and AGENTS.md to document the hardware profile system that replaces the baremetal-gpu topology file. Changes: - Remove baremetal-gpu as a separate clusterGroup - Document global.hardware.profile options (intel-tdx, amd-snp, intel-tdx-gpu, amd-snp-gpu) - Update bare metal deployment instructions to include hardware profile selection - Consolidate GPU deployment notes under baremetal section - Update AGENTS.md topology table to remove baremetal-gpu row Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add detect-hardware target to detect the appropriate hardware profile from cluster node labels set by NFD. Detects Intel TDX, AMD SEV-SNP, and NVIDIA GPU presence and recommends the correct hardware.profile value. Read-only/advisory — does not modify values-global.yaml automatically. Requires KUBECONFIG or oc login. Checks first node only (sufficient for SNO and homogeneous clusters). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Root cause identified: image-rs does not base64-decode keyData for cosign/sigstore. Passes raw base64 string to verifier instead of decoded PEM key. Bug location: image-rs/src/signature/policy/cosign/mod.rs:69 Current: key_data.as_bytes().to_vec() Should be: base64::decode(key_data)? Infrastructure ready to enable once upstream fix lands: - Sigstore key deployed to KBS - Policy templates configured - Verified working with podman (containers/image Go implementation) Blocked by: confidential-containers/image-rs (cosign keyData handling)
Upstream PR: confidential-containers/guest-components#1398 Root cause: image-rs does not base64-decode keyData for cosign/sigstore Infrastructure ready but waiting for fix in Red Hat build of trustee Covers: - Bug details and evidence - Infrastructure readiness (all deployed) - Re-enablement procedure - GPG vs sigstore comparison
Add PreSync hooks to all 4 CoCo workload charts (hello-openshift, kbs-access-curl, kbs-access-sealed, gpu-workload) to verify deployment dependencies before workload pods are created. Each chart now includes: - PreSync hook Job (sync-wave 9) that verifies: - inject-coco-initdata ClusterPolicy Ready - initdata-namespace-propagation ClusterPolicy Ready - initdata ConfigMap exists in target namespace - RBAC resources (sync-wave 0): - ServiceAccount (presync-verifier) - ClusterRole (read-only GET on ClusterPolicies and ConfigMaps) - ClusterRoleBinding Hook configuration: - Container: UBI9 (registry.access.redhat.com/ubi9/ubi:latest) - Timeout: 300s (activeDeadlineSeconds) - Retry: 3 attempts (backoffLimit) - Deletion policy: HookSucceeded,BeforeHookCreation This provides fail-fast behavior complementing Phase 1b's passive sync-wave ordering. If dependencies are not ready, the sync fails with clear error messages rather than deploying non-functional pods. Implementation: Phase 8 Plan 01 Reference: .planning/phases/08-argocd-sync-hooks-sequencing/08-01-PLAN.md Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
… policy names - Change hook type from PreSync to Sync (wave 10) - Remove sync-wave annotations from RBAC (deploy at wave 0) - Fix ClusterPolicy names to match actual policies - Fix kbs-access-sealed runtimeClassName template Root cause: PreSync hooks run before RBAC deploys (chicken-and-egg) Solution: Use Sync hooks at wave 10 (validated patterns best practice) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Change from ubi9/ubi to openshift/cli image - Remove kubectl download/extraction (oc already present) - Fixes tar permission denied error in Sync hooks Root cause: UBI9 runs as non-root, cannot write to /usr/local/bin Solution: Use image with oc pre-installed (validated patterns pattern) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Change from internal registry to registry.redhat.io - Fixes ImagePullBackOff: internal registry DNS not resolvable from hook pod - Uses registry.redhat.io/openshift4/ose-cli:latest (public, no auth needed for pull) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Change from .status.ready (null) to .status.conditions[?(@.type=="Ready")].status - Change comparison from "true" to "True" (capital T matches Kyverno format) - Fixes hook failure: ready field was empty, actual status is in conditions array Root cause: Kyverno ClusterPolicy status uses conditions[] array, not top-level ready field Solution: Use JSONPath filter to get Ready condition status Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…ock timing PROBLEM: Phase 8 added PreSync hooks that caused vp-gitops Application failure - presync-rbac.yaml created cluster-scoped ClusterRole/ClusterRoleBinding - Validated patterns framework expects child Applications to only create namespaced resources - vp-gitops failure prevented imperative namespace RBAC from being created - Without imperative-admin-sa, unsealvault-cronjob couldn't create pods (0 pods for 94+ min) - Vault remained locked, sealed secrets couldn't decrypt, workload pods failed ROOT CAUSE: Cluster-scoped RBAC in workload charts conflicts with clustergroup expectations SOLUTION: Replace PreSync hooks with ConfigMap volume mounts (upstream pattern design) 1. REMOVED 8 files: - presync-hook.yaml and presync-rbac.yaml from 4 charts (hello-openshift, kbs-access-curl, kbs-access-sealed, gpu-workload) - Eliminates cluster-scoped RBAC conflicts - Removes 20-32s sync delay (5-8s per hook × 4) 2. ADDED ConfigMap volume mounts to 5 Deployments: - hello-openshift/secure-deployment.yaml: mount initdata ConfigMap - hello-openshift/insecure-policy-deployment.yaml: mount debug-initdata ConfigMap - kbs-access-curl/deployment.yaml: mount debug-initdata ConfigMap - kbs-access-sealed/deployment.yaml: mount initdata ConfigMap - gpu-workload/gpu-vectoradd-deployment.yaml: mount debug-initdata ConfigMap - All mounts use 'optional: false' for kubelet enforcement 3. REMOVED sync-wave annotations: - Workload Deployments no longer use sync-wave: "10" - Application-level syncWave removed from values-baremetal.yaml (kbs-access-curl, kbs-access-sealed) - Matches upstream patterns (multicloud-gitops, industrial-edge) which use NO sync-waves on Deployments WHY THIS WORKS: - Kubelet enforces ConfigMap dependency: pod creation fails until ConfigMap exists - Kyverno ClusterPolicy (wave 1) propagates initdata before workloads attempt creation - Kubernetes built-in retry handles timing (no custom verification needed) - vp-gitops syncs successfully (no cluster-scoped conflicts) - imperative namespace RBAC is created, unsealvault-cronjob runs, vault unlocks - Defense in depth: annotation + volume mount + Kyverno validation + kubelet enforcement DESIGN RATIONALE: - Upstream validated patterns use ConfigMap volume mounts, NOT PreSync hooks - PreSync hooks are ONLY for RBAC setup at wave -15, not ConfigMap dependencies - Simpler to debug: pod events show "ConfigMap not found" vs opaque hook Job failures - Faster: no 5-8s sync delay per hook - Cleaner: no cluster-scoped RBAC per workload namespace Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
… reboot Wave 0: vault, eso, storage, kyverno (infrastructure first) Wave 10: baremetal, sandbox, nvidia-gpu (MCO-triggering, after vault has time to init/unseal) Wave 20: trustee, intel-dcap, sandbox-policies, coco-kyverno-policies (depends on vault secrets) Wave 30: hello-openshift, kbs-access-curl, kbs-access-sealed, gpu-workload (workloads last) Root cause: all apps deployed at wave 0 simultaneously, so sandbox/baremetal triggered MCO reboots before vault was initialized and unsealed. After reboot, secrets loading failed catastrophically. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
… sync serviceAccountCreate and adminServiceAccountCreate both created imperative-admin-sa (due to serviceAccountName override), causing ArgoCD "appeared 2 times" warning and refusing to sync the SA. This blocked unsealvault-cronjob from running (SA not found). Fix: set serviceAccountCreate: false since we only need the admin SA. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Adds a Sync hook Job at wave 5 that: 1. Polls vault status until Initialized=true, Sealed=false 2. Waits 120s for pattern.sh to finish loading secrets 3. Only then allows ArgoCD to proceed to wave 10+ apps This bridges the gap between "vault pod healthy" (wave 0) and "secrets loaded" which takes ~5 min after vault unseals. Without this, ArgoCD immediately creates wave 10 apps (sandbox, baremetal) which trigger MCO reboots before secrets are loaded. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
The sync hook pod runs as patterns-operator:default SA which lacks permissions to oc exec into vault namespace. Switch to curl against vault's internal service API (vault.vault.svc:8200/v1/sys/seal-status) which requires no RBAC. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…er patterns No validated pattern (multicloud-gitops, industrial-edge, medical-diagnosis) overrides serviceAccountName. Chart defaults create two SAs: - imperative-sa (read-only, used by CronJobs) - imperative-admin-sa (admin, for elevated tasks) Our override of serviceAccountName to imperative-admin-sa was non-standard and caused ArgoCD duplicate resource warnings. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…duplicate Research confirms serviceAccountCreate: false + adminServiceAccountCreate: true + serviceAccountName: imperative-admin-sa creates the SA exactly once. serviceAccountName is a reference, not a creator. The "appeared 2 times" ArgoCD warning was transient. CronJobs need admin SA for cluster-wide playbooks. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Replace python3 JSON parsing with simple grep for "sealed":false. ose-cli image may not have python3, and the parsing was failing silently. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…ccountName The serviceAccountCreate: false + serviceAccountName: imperative-admin-sa config consistently results in ArgoCD showing the SA as OutOfSync/Missing and refusing to create it. This blocks unsealvault-cronjob, which blocks vault initialization, which blocks the entire deployment. Use chart defaults: both imperative-sa and imperative-admin-sa get created. CronJobs run as imperative-sa (chart default). This matches every other validated pattern (multicloud-gitops, industrial-edge, medical-diagnosis). Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Chart defaults create both imperative-sa (read-only) and imperative-admin-sa (full admin). CronJobs need admin perms for vault init/unseal ansible playbooks. Override serviceAccountName to imperative-admin-sa while keeping both serviceAccountCreate and adminServiceAccountCreate at defaults (both true). Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Instead of overriding serviceAccountName to imperative-admin-sa (which causes ArgoCD "appeared 2 times" duplicate warning when both SAs have the same name), give imperative-sa full admin permissions via clusterRoleYaml override. CronJobs use imperative-sa (default) which now has all verbs on all resources. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…move stale sync-wave 1. insecure-policy: switch from origin-hello-openshift to ubi9/httpd-24 (same image as secure deployment, port 8080) 2. secure-configmap: remove unnecessary sync-wave annotation Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…bled
global.kata doesn't exist in any values file, causing helm template
to fail with "nil pointer evaluating interface {}.enabled". This chart
always runs with kata-cc runtime, so hardcode runtimeClassName directly.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
All CoCo workload charts now use global.coco.runtimeClassName instead of per-chart platform detection or hardcoded values. Defaults: kata-cc (values-global.yaml) Overrides: kata-remote (overrides/values-Azure.yaml, values-AWS.yaml) Exception: gpu-workload keeps kata-cc-nvidia-gpu via chart values.yaml Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Ensures Services, ConfigMaps, Secrets, and Routes sync before Deployments attempt to create pods. Prevents pods from failing due to missing dependencies during initial ArgoCD sync. Charts affected: hello-openshift (secure, insecure-policy), kbs-access-curl, kbs-access-sealed. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…56Mi) kata-cc pods require memory limit >= hypervisor default (2Gi) plus overhead. 256Mi was far too low and caused pod scheduling failures. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Only the limit needs to meet hypervisor minimum (2304Mi). Request stays low to avoid over-reserving node memory. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
CronJobs need admin perms for vault unseal and init-data-gzipper. Previous approach gave imperative-sa admin perms via clusterRoleYaml, which is non-standard. Instead, use the admin SA directly: - serviceAccountCreate: false (avoid duplicate SA with adminServiceAccountName) - serviceAccountName: imperative-admin-sa (CronJobs reference admin SA) The admin SA already has full cluster-admin perms via the upstream imperative-admin-cluster-role, so no clusterRoleYaml needed. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
With serviceAccountCreate: false, only one SA resource is rendered. If that single creation attempt hits the RBAC bootstrap race (ArgoCD controller lacks cluster-admin when SA is attempted), it fails with no retry until the full sync completes — causing a deadlock with manual install plan approval subscriptions. With serviceAccountCreate: true, two SA resources with the same name (imperative-admin-sa) are rendered. The duplicate is a cosmetic warning but doubles the creation attempts, making the race less likely to block. Upstream fix needed: sync-wave ordering on imperative templates and dedup when serviceAccountName == adminServiceAccountName. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
sandbox and trustee subscriptions have installPlanApproval: Manual, requiring the auto-approve-installplans CronJob to approve them. At wave 0, they create a deadlock: ArgoCD waits for subscription health → needs auto-approve CronJob → needs imperative-admin-sa → SA creation fails due to RBAC bootstrap race (CRB not yet applied). Moving these to wave 10 breaks the deadlock: wave 0 completes without waiting on Manual subscription health, SA retry succeeds once CRB propagates, and by wave 10 the CronJobs are running. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- standard: switch from origin-hello-openshift (port 8888) to httpd-24 (port 8080) with ConfigMap web content, matching the other deployments - insecure-policy: fix service and route targetPort from 8888 to 8080 to match httpd-24 container port (was returning 503) - secure: fix route targetPort from 8888 to 8080 to match service and container port (was returning 503) All three hello-openshift deployments now consistently use registry.redhat.io/ubi9/httpd-24 on port 8080. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
insecure-policy was serving the default httpd test page because it had no web content mounted at /var/www/html. Add a ConfigMap with custom index.html matching the pattern of standard and secure. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Document the route paths, security posture, and configuration for each CoCo workload chart: hello-openshift (3 deployments), kbs-access-curl (/secret.txt), and kbs-access-sealed (/secret-key). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.