Skip to content

Incremental changes for istiod#18132

Merged
istio-testing merged 15 commits intoistio:masterfrom
costinm:istiod3
Oct 25, 2019
Merged

Incremental changes for istiod#18132
istio-testing merged 15 commits intoistio:masterfrom
costinm:istiod3

Conversation

@costinm
Copy link
Copy Markdown
Contributor

@costinm costinm commented Oct 21, 2019

  • remove the SDS startup in pilot - the feedback is to start with SDS in
    agent, pilot will only sign certs.
  • cleanup a bit the startup
  • use port 8080 for http - istioctl expects it.
  • add the original Dockerfile - while it is getting integrated into the
    istio build system ( can be useful after as well, it's very easy)

Please provide a description for what this PR is for.

And to help us figure out who should review this PR, please
put an X in all the areas that this PR affects.

[ ] Configuration Infrastructure
[ ] Docs
[ ] Installation
[ ] Networking
[ ] Performance and Scalability
[ ] Policies and Telemetry
[ ] Security
[ ] Test and Release
[ ] User Experience
[ ] Developer Infrastructure

- remove the SDS startup in pilot - the feedback is to start with SDS in
agent, pilot will only sign certs.
- cleanup a bit the startup
- use port 8080 for http - istioctl expects it.
- add the original Dockerfile - while it is getting integrated into the
istio build system ( can be useful after as well, it's very easy)
@googlebot googlebot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Oct 21, 2019
@istio-testing istio-testing added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 21, 2019
@costinm costinm requested a review from a team as a code owner October 21, 2019 20:52
@howardjohn
Copy link
Copy Markdown
Member

/retest

@costinm costinm requested a review from a team October 22, 2019 21:45
@costinm costinm requested a review from a team as a code owner October 22, 2019 21:45
@istio-testing istio-testing added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 22, 2019
//RunValidation start running Galley validation mode
func RunValidation(ready chan<- struct{}, stopCh chan struct{}, vc *WebhookParameters, kubeConfig string,
livenessProbeController, readinessProbeController probe.Controller) {
func RunValidation(ready chan<- struct{}, stopCh chan struct{}, vc *WebhookParameters, kubeInterface *kubernetes.Clientset, kubeConfig string, livenessProbeController, readinessProbeController probe.Controller) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you replace kubeconfig with kubeInterface? We don't need to support both.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll leave it to a Galley expert - I tried to minimize risks to existing code paths ( which may eventually go away ). The kubeconfig string is used deep in the code.

I think the biggest issue is getting Interfaces() to use the kubernetes.Interface instead of the config, so a single client is used.

@costinm
Copy link
Copy Markdown
Contributor Author

costinm commented Oct 23, 2019

/retest

@costinm
Copy link
Copy Markdown
Contributor Author

costinm commented Oct 23, 2019

/test lint_istio

@costinm
Copy link
Copy Markdown
Contributor Author

costinm commented Oct 23, 2019

/test e2e-simpleTests-distroless_istio

@sbezverk
Copy link
Copy Markdown
Contributor

sbezverk commented Oct 24, 2019

@costinm I am a bit confused, I was under impression that no webhook related work has already being done, also I thought the direction was to have webhook as a separate package and call its functions from istiod while have webhook in Galley disabled.
Could you please keep work list and already taken tasks updated, so no stepping on other people toes would happen. I will pick some other task then..

@howardjohn
Copy link
Copy Markdown
Member

@costinm you have some merge conflicts

@costinm
Copy link
Copy Markdown
Contributor Author

costinm commented Oct 24, 2019

/retest

@istio-testing
Copy link
Copy Markdown
Collaborator

istio-testing commented Oct 24, 2019

@costinm: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
integ-new-install-k8s-tests_istio a081195 link /test integ-new-install-k8s-tests_istio
Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@costinm costinm mentioned this pull request Oct 25, 2019
@istio-testing istio-testing merged commit 5785312 into istio:master Oct 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants