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

Switch jinja2 to grpc based expansion #1885

Merged

Conversation

barney-s
Copy link
Collaborator

@barney-s barney-s commented May 23, 2024

Change description

Switch jinja2 to grpc based expansion:

  • rename gjinjg2 -> jinja2 (making jinja2 grpc based)
  • rename existing pod based jinja2 -> pjinja2
  • Add image field to ExpanderVersion
    • allow specifying an image that doesnt follow the
      removePrefix(expanderversion.name , "composition-") pattern.
    • provides more flexibility
    • In our case the type is pjinja and the image is expander-jinja
  • Add a custom ratelimiter for the facade reconciler
    • Due to a bug, the reconciler is not restarted across tests.
    • Old failures add to rate limiting causing random failures
    • make ratelimiter max backoff 120s
  • Add a small sleep b/w tests
  • Move all ExistTimeout to CompositionReconcileTimeout
  • Check if a plan was updated and if so check if the applier sees the
    latest plan object. If not retry again.
  • Add InputGeneration and Generation to plan status

@barney-s
Copy link
Collaborator Author

/assign @cheftako
/assign @xiaoweim

@barney-s barney-s changed the title Switch jinja2 to grpc based expansion WIP Switch jinja2 to grpc based expansion May 23, 2024
@barney-s barney-s force-pushed the switch-jinja2-grpc branch 8 times, most recently from 3ee932d to 532f4e7 Compare May 25, 2024 06:52
@barney-s barney-s changed the title WIP Switch jinja2 to grpc based expansion Switch jinja2 to grpc based expansion May 25, 2024
@barney-s barney-s force-pushed the switch-jinja2-grpc branch from 532f4e7 to 936ea11 Compare May 25, 2024 07:14
- rename gjinjg2 -> jinja2 (making jinja2 grpc based)
- rename existing pod based jinja2 -> pjinja2
- Add `image` field to ExpanderVersion
  - allow specifying an image that doesnt follow the
    removePrefix(expanderversion.name , "composition-") pattern.
  - provides more flexibility
  - In our case the type is pjinja and the image is expander-jinja
- Add a custom ratelimiter for the facade reconciler
  - Due to a bug, the reconciler is not restarted across tests.
  - Old failures add to rate limiting causing random failures
  - make ratelimiter max backoff 120s
- Add a small sleep b/w tests
- Move all ExistTimeout to CompositionReconcileTimeout
- Check if a plan was updated and if so check if the applier sees the
  latest plan object. If not retry again.
- Add InputGeneration and Generation to plan status
@barney-s barney-s force-pushed the switch-jinja2-grpc branch from 936ea11 to de42d6e Compare May 30, 2024 16:11
@xiaoweim
Copy link
Collaborator

/lgtm

@google-oss-prow google-oss-prow bot added the lgtm label May 30, 2024
@barney-s
Copy link
Collaborator Author

/approve

Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: barney-s

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

@google-oss-prow google-oss-prow bot merged commit 311592b into GoogleCloudPlatform:master May 30, 2024
6 checks passed
@barney-s barney-s deleted the switch-jinja2-grpc branch May 30, 2024 18:22
@yuwenma yuwenma added this to the 1.118 milestone Oct 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants