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

docker: remove nsswitch.conf #15155

Merged
merged 1 commit into from
Feb 2, 2023
Merged

Conversation

iavael
Copy link
Contributor

@iavael iavael commented Jan 20, 2023

Since docker images are not alpine-based anymore and do support nss, nsswitch.conf hack is not required anymore

@iavael iavael force-pushed the remove-nsswitch-conf branch from daf9db6 to 38ea543 Compare January 20, 2023 21:09
Copy link
Member

@ahrtr ahrtr left a comment

Choose a reason for hiding this comment

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

Good catch!

Thank you @iavael

Could you backport this PR to 3.5 and 3.4?

@serathius
Copy link
Member

Simplifying dockerfiles has been on my mind for long time. Thanks for looking into this!

@serathius
Copy link
Member

serathius commented Jan 21, 2023

@ahrtr There is no need to backport this. This is more of a cleanup. Let's not backport cleanups. I doesn't benefit older releases and would only confuse users reading changelog.

@ahrtr
Copy link
Member

ahrtr commented Jan 21, 2023

@ahrtr There is no need to backport this. This is more of a cleanup. Let's not backport cleanups. I doesn't benefit older releases and would only confuse users reading changelog.

Actually it's a minor issue (introduced in one of my previous PRs) to me. There are two duplicate keys "hosts", see line 21 and line 12.

hosts: files mdns4_minimal [NOTFOUND=return] dns mdns4

hosts: files dns

I suggest to backport the PR.

@ahrtr
Copy link
Member

ahrtr commented Jan 21, 2023

Since 3.4.24 isn't released yet, and so no need to update the changelog for 3.4. Note all dockerfile/nsswitch.conf changes will only be included in 3.4.24.

I agree we need to update 3.5 changelog after backporting.

@iavael
Copy link
Contributor Author

iavael commented Jan 22, 2023

@ahrtr

Could you backport this PR to 3.5 and 3.4?

PRs created

@ahrtr
Copy link
Member

ahrtr commented Jan 22, 2023

@ahrtr

Could you backport this PR to 3.5 and 3.4?

PRs created

Thank you @iavael . I see that you marked both PRs as draft, anything else you want to update on them? Both of them seems good to me. Please mark them as ready for review if you think they are ready.

@ahrtr
Copy link
Member

ahrtr commented Jan 22, 2023

Please also add a changelog item for 3.5 in CHANGELOG-3.5.md.

Since the PR adding nsswitch.conf isn't included in the latest 3.4 release (3.4.23) yet, so no need to add changelog item for 3.4.

@codecov-commenter
Copy link

Codecov Report

Merging #15155 (b1cddde) into main (ee566c4) will decrease coverage by 0.36%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main   #15155      +/-   ##
==========================================
- Coverage   74.93%   74.57%   -0.36%     
==========================================
  Files         415      415              
  Lines       34326    34326              
==========================================
- Hits        25721    25598     -123     
- Misses       6986     7089     +103     
- Partials     1619     1639      +20     
Flag Coverage Δ
all 74.57% <ø> (-0.36%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
server/proxy/grpcproxy/register.go 69.76% <0.00%> (-11.63%) ⬇️
server/storage/mvcc/watchable_store.go 84.58% <0.00%> (-9.68%) ⬇️
client/v3/experimental/recipes/queue.go 58.62% <0.00%> (-6.90%) ⬇️
client/v3/leasing/util.go 91.66% <0.00%> (-6.67%) ⬇️
client/pkg/v3/fileutil/purge.go 68.85% <0.00%> (-6.56%) ⬇️
client/v3/namespace/watch.go 87.87% <0.00%> (-6.07%) ⬇️
server/etcdserver/api/v3rpc/interceptor.go 72.39% <0.00%> (-5.21%) ⬇️
server/lease/lease.go 94.87% <0.00%> (-5.13%) ⬇️
client/pkg/v3/testutil/leak.go 62.83% <0.00%> (-4.43%) ⬇️
client/v3/concurrency/session.go 85.10% <0.00%> (-4.26%) ⬇️
... and 23 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

CHANGELOG/CHANGELOG-3.5.md Outdated Show resolved Hide resolved
@ahrtr
Copy link
Member

ahrtr commented Jan 23, 2023

3.5.7 is already released, so please add the item to 3.5.8.

Copy link
Member

@ahrtr ahrtr left a comment

Choose a reason for hiding this comment

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

Please rebase this PR and resolve the comment for changelog

@iavael iavael force-pushed the remove-nsswitch-conf branch from 1fc2a9e to 662a4c1 Compare January 25, 2023 14:58
CHANGELOG/CHANGELOG-3.5.md Outdated Show resolved Hide resolved
@ahrtr
Copy link
Member

ahrtr commented Jan 25, 2023

Please squash the commits into one.

@ahrtr
Copy link
Member

ahrtr commented Feb 1, 2023

@iavael please resolve the conflict and squash the commits.

@iavael iavael force-pushed the remove-nsswitch-conf branch from e76d7a9 to 9278066 Compare February 2, 2023 09:34
@ahrtr
Copy link
Member

ahrtr commented Feb 2, 2023

There is still conflict.

Co-authored-by: Benjamin Wang <wachao@vmware.com>
Signed-off-by: Iavael <905853+iavael@users.noreply.github.com>
@iavael iavael force-pushed the remove-nsswitch-conf branch from 9278066 to 9ce2b15 Compare February 2, 2023 11:08
Copy link
Member

@ahrtr ahrtr left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks @iavael

@ahrtr ahrtr merged commit dc2b198 into etcd-io:main Feb 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants