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

Skip resolvectl for now #591

Merged

Conversation

DimitriPapadopoulos
Copy link
Collaborator

@DimitriPapadopoulos DimitriPapadopoulos commented Mar 23, 2020

On Fedora resolvectl does not work as expected when invoked as resolvconf.
Also the exit status is not 0 so the current test never works actually:

$ resolvconf -a -f ; echo $?
Expected interface name as argument.
1
$ 

I suggest this goes in openfortivpn 1.13.2.

@DimitriPapadopoulos
Copy link
Collaborator Author

Instead implement #590.

Hopefully we will have some input from:
https://bugzilla.redhat.com/show_bug.cgi?id=1815605

On Fedora it does not work as expected when invoked as resolvconf.
Also the exit status is not 0 so the test does not work actually:

	$ resolvconf -a -f ; echo $?
	Expected interface name as argument.
	1
	$
@mrbaseman
Copy link
Collaborator

The intention actually was to catch this return value of 1. If resolvctl works as described in the man page it should support the -f option when called es resolvconf and silently ignore a missing interface. If it does not work properly, we should not use it.

But maybe I have misunderstood this sentence. Maybe "missing interface" means an interface name supplied to the call, but not describing an existing interface.

@DimitriPapadopoulos
Copy link
Collaborator Author

DimitriPapadopoulos commented Mar 23, 2020

Let me detail what happens on Fedora:

I may be wrong but I had understood that you were expecting the exit status of resolvconf -a -f to be 0 based on the resolvectl(1) man page. I would not necessarily interpret "will silently execute no operation" as meaning that the exit status is 0, just that no message is written to the terminal. In any case both assumptions are wrong without an interface name passed as an argument to -a: an error message is written to the terminal and the exist status is 1:

$ sudo resolvconf -a -f ; echo $?
Expected interface name as argument.
1
$ 
$ sudo resolvconf -d -f ; echo $?
Expected interface name as argument.
1
$ 

Unless I am missing something, the existing test attempts to catch an exit status of 0, not an exit status of 1, doesn't it?

Finally invoking resolvectl as resolvconf in "real life" doesn't seem to be working either, at least on Fedora, see:
https://bugzilla.redhat.com/show_bug.cgi?id=1815605

Running this as root on Fedora 31:

$ cat /etc/resolv.conf 
# Generated by NetworkManager
nameserver 10.0.2.3
$ 
$ cat MY_FILE 
namsserver 1.1.1.1
nameserver 1.0.0.1
$ 
$ cat MY_FILE | /usr/sbin/resolvconf -a enp0s3 -f ; echo $?
0
$ 
$ cat /etc/resolv.conf 
# Generated by NetworkManager
nameserver 10.0.2.3
$ 
$ cat MY_FILE | /usr/sbin/resolvconf -a enp0s3 ; echo $?
0
$ 
$ cat /etc/resolv.conf 
# Generated by NetworkManager
nameserver 10.0.2.3
$ 

@DimitriPapadopoulos
Copy link
Collaborator Author

DimitriPapadopoulos commented Mar 23, 2020

Ah, the exit status is 0 when using a fake interface name with -f - but of course not without -f:

$ /usr/sbin/resolvconf -a foobar -f ; echo $?
0
$ 
$ usr/sbin/resolvconf -a foobar; echo $?
Unknown interface 'foobar': No such device
1
$ 

That doesn't change the fact that invoking ressolvectl as resolvconf does not seem to be working.

Copy link
Collaborator

@mrbaseman mrbaseman left a comment

Choose a reason for hiding this comment

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

Based on the discussion I agree to skip the resolvectl case here. It does not work as expected and the configure test does not even catch the case that I was trying to address with it.

@DimitriPapadopoulos DimitriPapadopoulos merged commit 16c6ed3 into adrienverge:master Mar 23, 2020
@DimitriPapadopoulos DimitriPapadopoulos deleted the skip_resolvectl branch March 23, 2020 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants