Skip to content

Ingress Conversion should retain namespace #5818

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 8 commits into from
Closed

Ingress Conversion should retain namespace #5818

wants to merge 8 commits into from

Conversation

esnible
Copy link
Contributor

@esnible esnible commented May 24, 2018

Resolves Issue 5817

@linsun
Copy link
Member

linsun commented May 24, 2018

seems useful and low risk for 0.8, i'd vote to get it in @costinm

frankbu
frankbu approved these changes May 24, 2018
View reviewed changes
Copy link
Contributor

@frankbu frankbu left a comment

Choose a reason for hiding this comment

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

/lgtm

@istio-testing
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: esnible, frankbu
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: ayj

Assign the PR to them by writing /assign @ayj in a comment when ready.

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

@esnible
Copy link
Contributor Author

esnible commented May 24, 2018

/test istio-pilot-e2e
/test istio-pilot-e2e-envoyv2-v1alpha3

@esnible
Copy link
Contributor Author

esnible commented May 24, 2018

/test istio-pilot-e2e

@istio-testing
Copy link
Collaborator

New changes are detected. LGTM label has been removed.

@codecov
Copy link

codecov bot commented May 29, 2018 •
edited
Loading

Codecov Report

Merging #5818 into release-0.8 will increase coverage by 1%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##           release-0.8   #5818    +/-   ##
============================================
+ Coverage           73%     73%    +1%     
============================================
  Files              322     322            
  Lines            27776   27862    +86     
============================================
+ Hits             20136   20297   +161     
+ Misses            6846    6765    -81     
- Partials           794     800     +6
Impacted Files Coverage Δ
...ilot/pkg/networking/plugin/authn/authentication.go 68% <0%> (-15%) ⬇️
mixer/adapter/cloudwatch/cloudwatch.go 50% <0%> (-7%) ⬇️
mixer/adapter/statsd/statsd.go 94% <0%> (-3%) ⬇️
mixer/adapter/kubernetesenv/kubernetesenv.go 68% <0%> (-2%) ⬇️
mixer/adapter/servicecontrol/testhelper.go 72% <0%> (-1%) ⬇️
pilot/pkg/serviceregistry/aggregate/controller.go 70% <0%> (ø) ⬇️
pilot/pkg/config/clusterregistry/conversion.go 33% <0%> (ø) ⬇️
mixer/adapter/stackdriver/log/log.go 68% <0%> (ø) ⬇️
pilot/pkg/networking/core/v1alpha3/listener.go 1% <0%> (ø) ⬇️
...g/serviceregistry/cloudfoundry/servicediscovery.go 93% <0%> (ø) ⬇️
... and 24 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f7831f4...5f33263. Read the comment docs.

@esnible
Copy link
Contributor Author

esnible commented May 29, 2018

/retest

theganyo and others added 5 commits May 29, 2018 22:26
@theganyo @rshriram
use Gateway and VirtualService to expose helloworld service (istio#5791)
881a7a3 
* use Gateway and VirtualService to expose helloworld service

* readme fix
@gyliu513 @mandarjog
Make istio work without rbac. (istio#5877)
4490e22 
Fixed istio/old_issues_repo#255
@objectiser @mandarjog
Update Jaeger version and add limit on the number of traces held in m… (
9653fe9 
istio#5873)

* Update Jaeger version and add limit on the number of traces held in memory

* Use tag 1 to pick up latest stable versions

* Use tag 1.5
@mandarjog @rshriram
enable mixer alpha3_v2 tests (istio#5844)
e12e7bb 
* enable mixer alpha3_v2 tests

* fix capture logs for new target

* make rate limit test more resilient

* linter errors

* fix fmt

* update tests
@rkpagadala @hklai
update api sha to head of release-0.8 (istio#5939)
f7831f4
@esnible
Copy link
Contributor Author

esnible commented May 31, 2018

/test istio-presubmit

@linsun
Copy link
Member

linsun commented May 31, 2018

@costinm @hklai if there is another 0.8 build, I would like to get this in.

Every user will hit this when moving to 0.8 and use the converter tool.

@linsun
Copy link
Member

linsun commented Jun 1, 2018

ok opened a release note limitation for this. let's get this in for 0.8.1 @costin @hklai

istio/istio.io#1401

@esnible esnible changed the base branch from release-0.8 to master June 1, 2018 16:58
@esnible esnible mentioned this pull request Jun 1, 2018
Merged
@esnible
Copy link
Contributor Author

esnible commented Jun 1, 2018

Closing in favor of #5973 . That one was targeted against release-0.8 and had merge conflicts when I attempted to rebase against master.

@esnible esnible closed this Jun 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@linsun linsun linsun approved these changes

+1 more reviewer

@frankbu frankbu frankbu approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees

@frankbu frankbu

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@esnible @linsun @istio-testing @frankbu @googlebot @theganyo @gyliu513 @objectiser @mandarjog @rkpagadala