-
Notifications
You must be signed in to change notification settings - Fork 19
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
Create grpc deployment when starting operator #332
Open
troy0820
wants to merge
9
commits into
getporter:main
Choose a base branch
from
troy0820:troy0820/create-grpc-deployment
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+240
−0
Open
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
a637096
scaffold for deployment/service with flag for creation upon startup
troy0820 5eeb578
only create deployment if not found
troy0820 8bf94dd
add cm and log when complete
troy0820 16ee64d
create deployment spec
troy0820 9ed0cf7
create service,confimap and volumes for deployment
troy0820 8902e02
handle conflict if resource already exists
troy0820 14280fd
update manager role to allow creation of deployment, service and conf…
troy0820 3ec5f36
change image and command for grpc server
troy0820 1d5cecb
remove PorterNamespace and use const for image
troy0820 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
Oops, something went wrong.
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.
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.
This is where I think the complexity is for managing the grpc server as part of the controller. The porter config that this uses HAS to be the porter config that the installations used when running so that means it needs to use the porter config for the namespace. We put the restriction in the only allow a single porter config per namespace instead of a porter config to be defined for every installation. The operator sets up a default config if one doesn't exist. This service should be using the porter config that's applied to the namespace, either the default one or the user provided one. It also has to track if that PorterConfig ever changes and then reload with the new config so that it can hit the backing stores where the installations actually live.
Btw @schristoff we should add support for moving porter storage data from one backend to another. This will "just work" for secrets as long as the secrets are managed externally for the new store like they should be but for storage backend AFAIK there's no way to move from one DB to another if that changes in the porter config and we should definitely support that!
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.
That means we can't do this implementation because this is trying to install the grpc server before we even do an installation. If the installation needs to happen to success to then create a deployment/configmap/service to get the Porter config in the namespace, (that resource isn't created until something makes it get created) then we will have to do that after the first installation. Relying on the default Porter config at runtime seems a little tricky as that default is making assumption around the installation resource process.
What I can do is move this to be done once at setup during the first installation and dynamically create the configmap that will map to the porter config by the installation we have in the namespace once the installation is complete.
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.
This is really interesting because of how the Porter config gets resolved for an AgentAction:
operator/controllers/agentaction_controller.go
Line 554 in 0783aa4
We probably need to create a "default" grpc server in the operator namespace then have namespace specific servers IF a PorterConfig is specified for that namespace BUT not for the system..... This just feels gross.... But the resolvePorterConfig should be able to handle checking if a grpc server exists that will be able to handle the AgentAction based on the PorterConfig that is selected to run that AgentAction